diff --git a/AUTHORS b/AUTHORS index 76a383b..a0da6d61 100644 --- a/AUTHORS +++ b/AUTHORS
@@ -295,7 +295,6 @@ Jaideep Bajwa <bjaideep@ca.ibm.com> Jaime Soriano Pastor <jsorianopastor@gmail.com> Jake Helfert <jake@helfert.us> -Jake Hendy <me@jakehendy.com> Jakob Weigert <jakob.j.w@googlemail.com> Jakub Machacek <xtreit@gmail.com> James Choi <jchoi42@pha.jhu.edu>
diff --git a/DEPS b/DEPS index c675bee9..5ccb26b 100644 --- a/DEPS +++ b/DEPS
@@ -44,7 +44,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. - 'v8_revision': '68f430ce82eef2be84afb42cc7e70572ac10a23c', + 'v8_revision': '5bcc94934677532faaae0b9c2363e0358629c315', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling swarming_client # and whatever else without interference from each other.
diff --git a/base/android/java/src/org/chromium/base/ThreadUtils.java b/base/android/java/src/org/chromium/base/ThreadUtils.java index f95d9ac..dbad59d 100644 --- a/base/android/java/src/org/chromium/base/ThreadUtils.java +++ b/base/android/java/src/org/chromium/base/ThreadUtils.java
@@ -34,6 +34,11 @@ @VisibleForTesting public static void setUiThread(Looper looper) { synchronized (sLock) { + if (looper == null) { + // Used to reset the looper after tests. + sUiThreadHandler = null; + return; + } if (sUiThreadHandler != null && sUiThreadHandler.getLooper() != looper) { throw new RuntimeException("UI thread looper is already set to " + sUiThreadHandler.getLooper() + " (Main thread looper is "
diff --git a/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java b/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java index f833abba..9c16204c 100644 --- a/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java +++ b/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
@@ -13,6 +13,7 @@ import org.chromium.base.ContextUtils; import org.chromium.base.Log; import org.chromium.base.TraceEvent; +import org.chromium.base.VisibleForTesting; import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.MainDex; @@ -443,6 +444,15 @@ return sInstance.mLibraryProcessType; } + /** + * Override the library loader (normally with a mock) for testing. + * @param loader the mock library loader. + */ + @VisibleForTesting + public static void setLibraryLoaderForTesting(LibraryLoader loader) { + sInstance = loader; + } + private native void nativeInitCommandLine(String[] initCommandLine); // Only methods needed before or during normal JNI registration are during System.OnLoad.
diff --git a/chrome/android/BUILD.gn b/chrome/android/BUILD.gn index 0047020..0ceda16 100644 --- a/chrome/android/BUILD.gn +++ b/chrome/android/BUILD.gn
@@ -335,6 +335,7 @@ "//components/sync:sync_java_test_support", "//components/sync/android:sync_java", "//components/url_formatter/android:url_formatter_java", + "//components/variations/android:variations_java", "//components/web_restrictions:web_restrictions_java", "//content/public/android:content_java", "//device/geolocation:geolocation_java", @@ -342,6 +343,7 @@ "//third_party/WebKit/public:blink_headers_java", "//third_party/WebKit/public:mojo_bindings_java", "//third_party/android_tools:android_support_annotations_java", + "//third_party/android_tools:android_support_v7_appcompat_java", "//third_party/android_tools:android_support_v7_mediarouter_java", "//third_party/android_tools:android_support_v7_recyclerview_java", "//third_party/cacheinvalidation:cacheinvalidation_javalib",
diff --git a/chrome/android/java/AndroidManifest.xml b/chrome/android/java/AndroidManifest.xml index 8a76a7f..0980a89 100644 --- a/chrome/android/java/AndroidManifest.xml +++ b/chrome/android/java/AndroidManifest.xml
@@ -690,23 +690,6 @@ android:exported="false"> </service> - <service android:name="org.chromium.components.variations.firstrun.VariationsSeedService" - android:exported="false" /> - - <receiver android:name="org.chromium.components.variations.firstrun.VariationsSeedServiceLauncher" - android:permission="{{ manifest_package }}.TOS_ACKED"> - <intent-filter> - <action android:name="{{ manifest_package }}.TOS_ACKED" /> - </intent-filter> - </receiver> - - <receiver android:name="org.chromium.chrome.browser.firstrun.ToSAckedReceiver" - android:permission="{{ manifest_package }}.TOS_ACKED"> - <intent-filter> - <action android:name="{{ manifest_package }}.TOS_ACKED" /> - </intent-filter> - </receiver> - <!-- Bookmarks widget --> <receiver android:name="com.google.android.apps.chrome.appwidget.bookmarks.BookmarkThumbnailWidgetProvider" android:label="@string/bookmark_widget_title">
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeBackupAgent.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeBackupAgent.java index 2998638..6e40087 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeBackupAgent.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeBackupAgent.java
@@ -13,6 +13,7 @@ import org.chromium.base.ContextUtils; import org.chromium.base.Log; +import org.chromium.base.PathUtils; import org.chromium.base.ThreadUtils; import org.chromium.base.VisibleForTesting; import org.chromium.base.annotations.SuppressFBWarnings; @@ -20,6 +21,7 @@ import org.chromium.chrome.browser.firstrun.FirstRunGlueImpl; import org.chromium.chrome.browser.firstrun.FirstRunSignInProcessor; import org.chromium.chrome.browser.firstrun.FirstRunStatus; +import org.chromium.chrome.browser.init.AsyncInitTaskRunner; import org.chromium.chrome.browser.init.ChromeBrowserInitializer; import org.chromium.chrome.browser.preferences.privacy.PrivacyPreferencesManager; import org.chromium.components.signin.AccountManagerHelper; @@ -33,6 +35,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; /** * Backup agent for Chrome, using Android key/value backup. @@ -54,6 +58,10 @@ PrivacyPreferencesManager.PREF_METRICS_REPORTING, }; + // Timeout for running the background tasks, needs to be quite long since they may be doing + // network access, but must be less than the 1 minute restore timeout to be useful. + private static final long BACKGROUND_TASK_TIMEOUT_SECS = 20; + /** * Class to save and restore the backup state, used to decide if backups are needed. Since the * backup data is small, and stored as private data by the backup service, this can simply store @@ -111,7 +119,7 @@ try { ChromeBrowserInitializer.getInstance(context).handleSynchronousStartup(); } catch (ProcessInitException e) { - Log.w(TAG, "Browser launch failed on restore: " + e); + Log.w(TAG, "Browser launch failed on backup or restore: " + e); return false; } return true; @@ -134,27 +142,29 @@ final ArrayList<byte[]> backupValues = new ArrayList<>(); // The native preferences can only be read on the UI thread. - if (!ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() { - @Override - public Boolean call() { - // Start the browser if necessary, so that Chrome can access the native - // preferences. Although Chrome requests the backup, it doesn't happen - // immediately, so by the time it does Chrome may not be running. - if (!initializeBrowser(backupAgent)) return false; + Boolean nativePrefsRead = + ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() { + @Override + public Boolean call() { + // Start the browser if necessary, so that Chrome can access the native + // preferences. Although Chrome requests the backup, it doesn't happen + // immediately, so by the time it does Chrome may not be running. + if (!initializeBrowser(backupAgent)) return false; - String[] nativeBackupNames = nativeGetBoolBackupNames(); - boolean[] nativeBackupValues = nativeGetBoolBackupValues(); - assert nativeBackupNames.length == nativeBackupValues.length; + String[] nativeBackupNames = nativeGetBoolBackupNames(); + boolean[] nativeBackupValues = nativeGetBoolBackupValues(); + assert nativeBackupNames.length == nativeBackupValues.length; - for (String name : nativeBackupNames) { - backupNames.add(NATIVE_PREF_PREFIX + name); + for (String name : nativeBackupNames) { + backupNames.add(NATIVE_PREF_PREFIX + name); + } + for (boolean val : nativeBackupValues) { + backupValues.add(booleanToBytes(val)); + } + return true; } - for (boolean val : nativeBackupValues) { - backupValues.add(booleanToBytes(val)); - } - return true; - } - })) { + }); + if (!nativePrefsRead) { // Something went wrong reading the native preferences, skip the backup. return; } @@ -201,6 +211,9 @@ @Override public void onRestore(BackupDataInput data, int appVersionCode, ParcelFileDescriptor newState) throws IOException { + // TODO(aberent) Check that this is not running on the UI thread. Doing so, however, makes + // testing difficult since the test code runs on the UI thread. + // Check that the user hasn't already seen FRE (not sure if this can ever happen, but if it // does then restoring the backup will overwrite the user's choices). SharedPreferences sharedPrefs = ContextUtils.getAppSharedPreferences(); @@ -227,16 +240,45 @@ backupValues.add(buffer); } } + // Start and wait for the Async init tasks. This loads the library, and attempts to load the + // first run variations seed. Since these are both slow it makes sense to run them in + // parallel as Android AsyncTasks, reusing some of Chrome's async startup logic. + // + // Note that this depends on onRestore being run from a background thread, since + // if it were called from the UI thread the broadcast would not be received until after it + // exited. + final CountDownLatch latch = new CountDownLatch(1); + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + // Chrome library loading depends on PathUtils. + PathUtils.setPrivateDataDirectorySuffix( + ChromeBrowserInitializer.PRIVATE_DATA_DIRECTORY_SUFFIX); + createAsyncInitTaskRunner(latch).startBackgroundTasks( + false /* allocateChildConnection */, true /* initVariationSeed */); + } + }); - // Chrome has to be running before it can check if the account exists. + try { + // Ignore result. It will only be false if it times out. Problems with fetching the + // variation seed can be ignored, and other problems will either recover or be repeated + // when Chrome is started synchronously. + latch.await(BACKGROUND_TASK_TIMEOUT_SECS, TimeUnit.SECONDS); + } catch (InterruptedException e) { + // Should never happen, but can be ignored (as explained above) anyway. + } + // Chrome has to be running before it can check if the account exists. Because the native + // library is already loaded Chrome startup should be fast. final ChromeBackupAgent backupAgent = this; - if (!ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() { - @Override - public Boolean call() { - // Start the browser if necessary. - return initializeBrowser(backupAgent); - } - })) { + boolean browserStarted = + ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() { + @Override + public Boolean call() { + // Start the browser if necessary. + return initializeBrowser(backupAgent); + } + }); + if (!browserStarted) { // Something went wrong starting Chrome, skip the restore. return; } @@ -293,6 +335,24 @@ } @VisibleForTesting + AsyncInitTaskRunner createAsyncInitTaskRunner(final CountDownLatch latch) { + return new AsyncInitTaskRunner() { + + @Override + protected void onSuccess() { + latch.countDown(); + } + + @Override + protected void onFailure() { + // Ignore failure. Problems with the variation seed can be ignored, and other + // problems will either recover or be repeated when Chrome is started synchronously. + latch.countDown(); + } + }; + } + + @VisibleForTesting protected native String[] nativeGetBoolBackupNames(); @VisibleForTesting
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/init/AsyncInitTaskRunner.java b/chrome/android/java/src/org/chromium/chrome/browser/init/AsyncInitTaskRunner.java new file mode 100644 index 0000000..4ad3136d --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/init/AsyncInitTaskRunner.java
@@ -0,0 +1,134 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.init; + +import android.os.AsyncTask; + +import org.chromium.base.ContextUtils; +import org.chromium.base.ThreadUtils; +import org.chromium.base.VisibleForTesting; +import org.chromium.base.library_loader.LibraryLoader; +import org.chromium.base.library_loader.LibraryProcessType; +import org.chromium.base.library_loader.ProcessInitException; +import org.chromium.chrome.browser.ChromeVersionInfo; +import org.chromium.components.variations.firstrun.VariationsSeedFetcher; +import org.chromium.content.browser.ChildProcessLauncher; + +import java.util.concurrent.Executor; + +/** + * Runs asynchronous startup task that need to be run before the native side is + * started. Currently it runs two tasks: + * - Native library loading + * - Fetching the variations seed on first run + */ +public abstract class AsyncInitTaskRunner { + private boolean mFetchingVariations; + private boolean mLibraryLoaded; + + private LoadTask mLoadTask; + private FetchSeedTask mFetchSeedTask; + + @VisibleForTesting + boolean shouldFetchVariationsSeedDuringFirstRun() { + return ChromeVersionInfo.isOfficialBuild(); + } + + private class LoadTask extends AsyncTask<Boolean, Void, Boolean> { + @Override + protected Boolean doInBackground(Boolean... allocateChildConnection) { + try { + LibraryLoader libraryLoader = LibraryLoader.get(LibraryProcessType.PROCESS_BROWSER); + libraryLoader.ensureInitialized(); + // The prefetch is done after the library load for two reasons: + // - It is easier to know the library location after it has + // been loaded. + // - Testing has shown that this gives the best compromise, + // by avoiding performance regression on any tested + // device, and providing performance improvement on + // some. Doing it earlier delays UI inflation and more + // generally startup on some devices, most likely by + // competing for IO. + // For experimental results, see http://crbug.com/460438. + libraryLoader.asyncPrefetchLibrariesToMemory(); + } catch (ProcessInitException e) { + return false; + } + if (allocateChildConnection[0]) { + ChildProcessLauncher.warmUp(ContextUtils.getApplicationContext()); + } + return true; + } + + @Override + protected void onPostExecute(Boolean result) { + mLibraryLoaded = result; + tasksPossiblyComplete(mLibraryLoaded); + } + } + + private class FetchSeedTask extends AsyncTask<Void, Void, Void> { + @Override + protected Void doInBackground(Void... params) { + VariationsSeedFetcher.get().fetchSeed(); + return null; + } + + @Override + protected void onPostExecute(Void result) { + mFetchingVariations = false; + tasksPossiblyComplete(true); + } + } + + /** + * Starts the background tasks. + * @param allocateChildConnection Whether a spare child connection should be allocated. Set to + * false if you know that no new renderer is needed. + * @param fetchVariationSeed Whether to initialize the variations seed, if it hasn't been + * initialized in a previous run. + */ + public void startBackgroundTasks(boolean allocateChildConnection, boolean fetchVariationSeed) { + ThreadUtils.assertOnUiThread(); + assert mLoadTask == null; + if (fetchVariationSeed && shouldFetchVariationsSeedDuringFirstRun()) { + mFetchingVariations = true; + mFetchSeedTask = new FetchSeedTask(); + mFetchSeedTask.executeOnExecutor(getExecutor()); + } + + mLoadTask = new LoadTask(); + mLoadTask.executeOnExecutor(getExecutor(), allocateChildConnection); + } + + private void tasksPossiblyComplete(boolean result) { + ThreadUtils.assertOnUiThread(); + + if (!result) { + mLoadTask.cancel(true); + if (mFetchSeedTask != null) mFetchSeedTask.cancel(true); + onFailure(); + } + + if (mLibraryLoaded && !mFetchingVariations) { + onSuccess(); + } + } + + @VisibleForTesting + protected Executor getExecutor() { + return AsyncTask.THREAD_POOL_EXECUTOR; + } + + /** + * Handle successful completion of the Async initialization tasks. + */ + protected abstract void onSuccess(); + + /** + * Handle failed completion of the Async initialization tasks. + */ + protected abstract void onFailure(); +}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/init/NativeInitializationController.java b/chrome/android/java/src/org/chromium/chrome/browser/init/NativeInitializationController.java index 09d44e5..5cc01c8 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/init/NativeInitializationController.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/init/NativeInitializationController.java
@@ -4,13 +4,9 @@ package org.chromium.chrome.browser.init; -import android.content.BroadcastReceiver; -import android.content.Context; import android.content.Intent; -import android.content.IntentFilter; import android.os.Handler; import android.os.Looper; -import android.support.v4.content.LocalBroadcastManager; import android.util.Log; import org.chromium.base.ContextUtils; @@ -18,10 +14,7 @@ import org.chromium.base.library_loader.LibraryLoader; import org.chromium.base.library_loader.LibraryProcessType; import org.chromium.base.library_loader.ProcessInitException; -import org.chromium.chrome.browser.ChromeVersionInfo; import org.chromium.chrome.browser.firstrun.FirstRunFlowSequencer; -import org.chromium.components.variations.firstrun.VariationsSeedService; -import org.chromium.content.browser.ChildProcessLauncher; import java.util.ArrayList; import java.util.List; @@ -43,9 +36,8 @@ private List<Intent> mPendingNewIntents; private List<ActivityResult> mPendingActivityResults; - private boolean mLibraryLoaded; + private boolean mBackgroundTasksComplete; private boolean mHasDoneFirstDraw; - private boolean mWaitingForVariationsFetch; private boolean mHasSignaledLibraryLoaded; private boolean mInitializationComplete; @@ -85,77 +77,38 @@ public void startBackgroundTasks(final boolean allocateChildConnection) { ThreadUtils.assertOnUiThread(); - // TODO(asvitkine): Consider moving this logic to a singleton, like - // ChromeBrowserInitializer. - if (ChromeVersionInfo.isOfficialBuild()) { - Context context = ContextUtils.getApplicationContext(); - Intent initialIntent = mActivityDelegate.getInitialIntent(); - if (FirstRunFlowSequencer.checkIfFirstRunIsNecessary(context, initialIntent, false) - != null) { - mWaitingForVariationsFetch = true; - IntentFilter filter = new IntentFilter(VariationsSeedService.COMPLETE_BROADCAST); - final LocalBroadcastManager manager = LocalBroadcastManager.getInstance(context); - manager.registerReceiver( - new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - // This check is needed because onReceive() can be called multiple - // times even after having unregistered below if two broadcasts - // arrive in rapid succession. - if (!mWaitingForVariationsFetch) return; - mWaitingForVariationsFetch = false; - manager.unregisterReceiver(this); - signalNativeLibraryLoadedIfReady(); - } - }, - filter); - context.startService(new Intent(context, VariationsSeedService.class)); - } - } + boolean fetchVariationsSeed = FirstRunFlowSequencer.checkIfFirstRunIsNecessary( + ContextUtils.getApplicationContext(), + mActivityDelegate.getInitialIntent(), false) + != null; - // TODO(yusufo) : Investigate using an AsyncTask for this. - new Thread() { + mBackgroundTasksComplete = false; + new AsyncInitTaskRunner() { + @Override - public void run() { - try { - LibraryLoader libraryLoader = - LibraryLoader.get(LibraryProcessType.PROCESS_BROWSER); - libraryLoader.ensureInitialized(); - // The prefetch is done after the library load for two reasons: - // - It is easier to know the library location after it has - // been loaded. - // - Testing has shown that this gives the best compromise, - // by avoiding performance regression on any tested - // device, and providing performance improvement on - // some. Doing it earlier delays UI inflation and more - // generally startup on some devices, most likely by - // competing for IO. - // For experimental results, see http://crbug.com/460438. - libraryLoader.asyncPrefetchLibrariesToMemory(); - } catch (ProcessInitException e) { - Log.e(TAG, "Unable to load native library.", e); - mActivityDelegate.onStartupFailure(); - return; - } - if (allocateChildConnection) { - ChildProcessLauncher.warmUp(ContextUtils.getApplicationContext()); - } - ThreadUtils.runOnUiThread(new Runnable() { - @Override - public void run() { - mLibraryLoaded = true; - signalNativeLibraryLoadedIfReady(); - } - }); + protected void onSuccess() { + ThreadUtils.assertOnUiThread(); + + mBackgroundTasksComplete = true; + signalNativeLibraryLoadedIfReady(); } - }.start(); + + @Override + protected void onFailure() { + // Initialization has failed, call onStartup failure to abandon the activity. + // This is not expected to return, so there is no need to set + // mBackgroundTasksComplete or do any other tidying up. + mActivityDelegate.onStartupFailure(); + } + + }.startBackgroundTasks(allocateChildConnection, fetchVariationsSeed); } private void signalNativeLibraryLoadedIfReady() { ThreadUtils.assertOnUiThread(); // Called on UI thread when any of the booleans below have changed. - if (mHasDoneFirstDraw && mLibraryLoaded && !mWaitingForVariationsFetch) { + if (mHasDoneFirstDraw && mBackgroundTasksComplete) { // This block should only be hit once. assert !mHasSignaledLibraryLoaded; mHasSignaledLibraryLoaded = true;
diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni index 9cf15b2..3a985fbb 100644 --- a/chrome/android/java_sources.gni +++ b/chrome/android/java_sources.gni
@@ -452,6 +452,7 @@ "java/src/org/chromium/chrome/browser/infobar/TranslateSubPanel.java", "java/src/org/chromium/chrome/browser/infobar/UpdatePasswordInfoBar.java", "java/src/org/chromium/chrome/browser/init/AsyncInitializationActivity.java", + "java/src/org/chromium/chrome/browser/init/AsyncInitTaskRunner.java", "java/src/org/chromium/chrome/browser/init/BrowserParts.java", "java/src/org/chromium/chrome/browser/init/ChromeActivityNativeDelegate.java", "java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java", @@ -1511,6 +1512,7 @@ "junit/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencerTest.java", "junit/src/org/chromium/chrome/browser/fullscreen/BrowserStateBrowserControlsVisibilityDelegateTest.java", "junit/src/org/chromium/chrome/browser/gcore/GoogleApiClientHelperTest.java", + "junit/src/org/chromium/chrome/browser/init/AsyncInitTaskRunnerTest.java", "junit/src/org/chromium/chrome/browser/invalidation/InvalidationControllerTest.java", "junit/src/org/chromium/chrome/browser/media/remote/AbstractMediaRouteControllerTest.java", "junit/src/org/chromium/chrome/browser/media/remote/MediaUrlResolverTest.java",
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ChromeBackupAgentTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/ChromeBackupAgentTest.java index 3ae9e03..5361ae4a 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/ChromeBackupAgentTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/ChromeBackupAgentTest.java
@@ -12,7 +12,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -38,9 +37,11 @@ import org.chromium.base.BaseChromiumApplication; import org.chromium.base.ContextUtils; +import org.chromium.base.PathUtils; import org.chromium.base.library_loader.ProcessInitException; import org.chromium.chrome.browser.firstrun.FirstRunSignInProcessor; import org.chromium.chrome.browser.firstrun.FirstRunStatus; +import org.chromium.chrome.browser.init.AsyncInitTaskRunner; import org.chromium.components.signin.ChromeSigninController; import org.chromium.testing.local.LocalRobolectricTestRunner; @@ -51,6 +52,7 @@ import java.io.ObjectInputStream; import java.util.ArrayList; import java.util.Arrays; +import java.util.concurrent.CountDownLatch; /** * Unit tests for {@link org.chromium.chrome.browser.ChromeBackupAgent}. @@ -60,6 +62,7 @@ public class ChromeBackupAgentTest { private Context mContext; private ChromeBackupAgent mAgent; + private AsyncInitTaskRunner mTaskRunner; private void setUpTestPrefs(SharedPreferences prefs) { SharedPreferences.Editor editor = prefs.edit(); @@ -70,20 +73,39 @@ } @Before - public void setUp() throws ProcessInitException { + public void setUp() { // Set up the context. mContext = RuntimeEnvironment.application.getApplicationContext(); ContextUtils.initApplicationContextForTests(mContext); - // Override the native calls. - mAgent = spy(new ChromeBackupAgent()); - doReturn(new String[] {"pref1"}).when(mAgent).nativeGetBoolBackupNames(); - doReturn(new boolean[] {true}).when(mAgent).nativeGetBoolBackupValues(); - doNothing().when(mAgent).nativeSetBoolBackupPrefs( - any(String[].class), any(boolean[].class)); + // Create the agent to test; override the native calls and fetching the task runner, and + // spy on the agent to allow us to validate calls to these methods. + mAgent = spy(new ChromeBackupAgent() { + @Override + AsyncInitTaskRunner createAsyncInitTaskRunner(CountDownLatch latch) { + latch.countDown(); + return mTaskRunner; + } + + @Override + protected String[] nativeGetBoolBackupNames() { + return new String[] {"pref1"}; + } + + @Override + protected boolean[] nativeGetBoolBackupValues() { + return new boolean[] {true}; + } + + @Override + protected void nativeSetBoolBackupPrefs(String[] s, boolean[] b) {} + }); // Mock initializing the browser doReturn(true).when(mAgent).initializeBrowser(any(Context.class)); + + // Mock the AsyncTaskRunner. + mTaskRunner = mock(AsyncInitTaskRunner.class); } /** @@ -309,7 +331,6 @@ @Override public Integer answer(InvocationOnMock invocation) throws Throwable { - // TODO(aberent): Auto-generated method stub byte[] buffer = invocation.getArgument(0); for (int i = 0; i < values[mPos].length; i++) { buffer[i] = values[mPos][i]; @@ -334,18 +355,20 @@ * * @throws IOException * @throws ClassNotFoundException + * @throws ProcessInitException + * @throws InterruptedException */ @Test - public void testOnRestore_normal() throws IOException, ClassNotFoundException { - BackupDataInput backupData = createMockBackupData(); - - doReturn(true).when(mAgent).accountExistsOnDevice(any(String.class)); - + public void testOnRestore_normal() + throws IOException, ClassNotFoundException, ProcessInitException, InterruptedException { // Create a state file. File stateFile = File.createTempFile("Test", ""); ParcelFileDescriptor newState = ParcelFileDescriptor.open(stateFile, ParcelFileDescriptor.parseMode("w")); + BackupDataInput backupData = createMockBackupData(); + doReturn(true).when(mAgent).accountExistsOnDevice(any(String.class)); + // Do a restore. mAgent.onRestore(backupData, 0, newState); SharedPreferences prefs = ContextUtils.getAppSharedPreferences(); @@ -353,6 +376,13 @@ assertFalse(prefs.contains("junk")); verify(mAgent).nativeSetBoolBackupPrefs( new String[] {"pref1", "pref2"}, new boolean[] {false, true}); + verify(mTaskRunner) + .startBackgroundTasks( + false /* allocateChildConnection */, true /* initVariationSeed */); + // The test mocks out everything that forces the AsyncTask used by PathUtils setup to + // complete. If it isn't completed before the test exits Robolectric crashes with a null + // pointer exception (although the test passes). Force it to complete by getting some data. + PathUtils.getDataDirectory(); } /** @@ -361,22 +391,30 @@ * * @throws IOException * @throws ClassNotFoundException + * @throws ProcessInitException */ @Test - public void testOnRestore_badUser() throws IOException, ClassNotFoundException { - BackupDataInput backupData = createMockBackupData(); - - doReturn(false).when(mAgent).accountExistsOnDevice(any(String.class)); - + public void testOnRestore_badUser() + throws IOException, ClassNotFoundException, ProcessInitException { // Create a state file. File stateFile = File.createTempFile("Test", ""); ParcelFileDescriptor newState = ParcelFileDescriptor.open(stateFile, ParcelFileDescriptor.parseMode("w")); + BackupDataInput backupData = createMockBackupData(); + doReturn(false).when(mAgent).accountExistsOnDevice(any(String.class)); + // Do a restore. mAgent.onRestore(backupData, 0, newState); SharedPreferences prefs = ContextUtils.getAppSharedPreferences(); assertFalse(prefs.contains(FirstRunStatus.FIRST_RUN_FLOW_COMPLETE)); verify(mAgent, never()).nativeSetBoolBackupPrefs(any(String[].class), any(boolean[].class)); + verify(mTaskRunner) + .startBackgroundTasks( + false /* allocateChildConnection */, true /* initVariationSeed */); + // The test mocks out everything that forces the AsyncTask used by PathUtils setup to + // complete. If it isn't completed before the test exits Robolectric crashes with a null + // pointer exception (although the test passes). Force it to complete by getting some data. + PathUtils.getDataDirectory(); } }
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/init/AsyncInitTaskRunnerTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/init/AsyncInitTaskRunnerTest.java new file mode 100644 index 0000000..5835abf --- /dev/null +++ b/chrome/android/junit/src/org/chromium/chrome/browser/init/AsyncInitTaskRunnerTest.java
@@ -0,0 +1,121 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.init; + +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.Robolectric; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.util.concurrent.RoboExecutorService; + +import org.chromium.base.ContextUtils; +import org.chromium.base.library_loader.LibraryLoader; +import org.chromium.base.library_loader.LibraryProcessType; +import org.chromium.base.library_loader.LoaderErrors; +import org.chromium.base.library_loader.ProcessInitException; +import org.chromium.components.variations.firstrun.VariationsSeedFetcher; +import org.chromium.testing.local.LocalRobolectricTestRunner; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; + +/** + * Tests for {@link AsyncInitTaskRunner} + */ +@RunWith(LocalRobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class AsyncInitTaskRunnerTest { + private static final int THREAD_WAIT_TIME_MS = 1000; + + private LibraryLoader mLoader; + private AsyncInitTaskRunner mRunner; + private CountDownLatch mLatch; + + private VariationsSeedFetcher mVariationsSeedFetcher; + + public AsyncInitTaskRunnerTest() throws ProcessInitException { + mLoader = spy(LibraryLoader.get(LibraryProcessType.PROCESS_BROWSER)); + doNothing().when(mLoader).ensureInitialized(); + doNothing().when(mLoader).asyncPrefetchLibrariesToMemory(); + LibraryLoader.setLibraryLoaderForTesting(mLoader); + mVariationsSeedFetcher = mock(VariationsSeedFetcher.class); + VariationsSeedFetcher.setVariationsSeedFetcherForTesting(mVariationsSeedFetcher); + ContextUtils.initApplicationContextForTests(RuntimeEnvironment.application); + + mLatch = new CountDownLatch(1); + mRunner = spy(new AsyncInitTaskRunner() { + @Override + protected void onSuccess() { + mLatch.countDown(); + } + @Override + protected void onFailure() { + mLatch.countDown(); + } + @Override + protected Executor getExecutor() { + return new RoboExecutorService(); + } + }); + // Allow test to run on all builds + when(mRunner.shouldFetchVariationsSeedDuringFirstRun()).thenReturn(true); + } + + @Test + public void libraryLoaderOnlyTest() + throws InterruptedException, ProcessInitException, ExecutionException { + mRunner.startBackgroundTasks(false, false); + + Robolectric.flushBackgroundThreadScheduler(); + Robolectric.flushForegroundThreadScheduler(); + assertTrue(mLatch.await(0, TimeUnit.SECONDS)); + verify(mLoader).ensureInitialized(); + verify(mLoader).asyncPrefetchLibrariesToMemory(); + verify(mRunner).onSuccess(); + verify(mVariationsSeedFetcher, never()).fetchSeed(); + } + + @Test + public void libraryLoaderFailTest() throws InterruptedException, ProcessInitException { + doThrow(new ProcessInitException(LoaderErrors.LOADER_ERROR_NATIVE_LIBRARY_LOAD_FAILED)) + .when(mLoader) + .ensureInitialized(); + mRunner.startBackgroundTasks(false, false); + + Robolectric.flushBackgroundThreadScheduler(); + Robolectric.flushForegroundThreadScheduler(); + assertTrue(mLatch.await(0, TimeUnit.SECONDS)); + verify(mRunner).onFailure(); + verify(mVariationsSeedFetcher, never()).fetchSeed(); + } + + @Test + public void fetchVariationsTest() throws InterruptedException, ProcessInitException { + mRunner.startBackgroundTasks(false, true); + + Robolectric.flushBackgroundThreadScheduler(); + Robolectric.flushForegroundThreadScheduler(); + assertTrue(mLatch.await(0, TimeUnit.SECONDS)); + verify(mLoader).ensureInitialized(); + verify(mLoader).asyncPrefetchLibrariesToMemory(); + verify(mRunner).onSuccess(); + verify(mVariationsSeedFetcher).fetchSeed(); + } + + // TODO(aberent) Test for allocateChildConnection. Needs refactoring of ChildProcessLauncher to + // make it mockable. +}
diff --git a/chrome/browser/chrome_browser_field_trials.cc b/chrome/browser/chrome_browser_field_trials.cc index 313bff3..69fac07e 100644 --- a/chrome/browser/chrome_browser_field_trials.cc +++ b/chrome/browser/chrome_browser_field_trials.cc
@@ -73,9 +73,7 @@ const uint32_t kAllocId = 0x935DDD43; // SHA1(BrowserMetrics) std::string storage = variations::GetVariationParamValueByFeature( base::kPersistentHistogramsFeature, "storage"); - - // As of M58, "MappedFile" is the default. - if (storage.empty() || storage == "MappedFile") { + if (storage == "MappedFile") { // If for some reason the existing "active" file could not be moved above // then it is essential it be scheduled for deletion when possible and the // contents ignored. Because this shouldn't happen but can on an OS like
diff --git a/chrome/common/chrome_features.cc b/chrome/common/chrome_features.cc index 809f2f5..fe299e5 100644 --- a/chrome/common/chrome_features.cc +++ b/chrome/common/chrome_features.cc
@@ -167,6 +167,9 @@ const base::Feature kOverrideYouTubeFlashEmbed{ "OverrideYouTubeFlashEmbed", base::FEATURE_ENABLED_BY_DEFAULT}; +const base::Feature kParallelDownloading{ + "ParallelDownloading", base::FEATURE_DISABLED_BY_DEFAULT}; + // Enables Permissions Blacklisting via Safe Browsing. const base::Feature kPermissionsBlacklist{ "PermissionsBlacklist", base::FEATURE_DISABLED_BY_DEFAULT};
diff --git a/chrome/common/chrome_features.h b/chrome/common/chrome_features.h index 430dd0d6..4a07739 100644 --- a/chrome/common/chrome_features.h +++ b/chrome/common/chrome_features.h
@@ -99,6 +99,8 @@ extern const base::Feature kOverrideYouTubeFlashEmbed; +extern const base::Feature kParallelDownloading; + extern const base::Feature kPermissionsBlacklist; #if BUILDFLAG(ENABLE_PLUGINS)
diff --git a/components/reading_list/ios/offline_url_utils.cc b/components/reading_list/ios/offline_url_utils.cc index a0045ef..25b0561b 100644 --- a/components/reading_list/ios/offline_url_utils.cc +++ b/components/reading_list/ios/offline_url_utils.cc
@@ -10,6 +10,7 @@ namespace { const char kOfflineDirectory[] = "Offline"; const char kMainPageFileName[] = "page.html"; +const char kPDFFileName[] = "file.pdf"; } // namespace namespace reading_list { @@ -22,20 +23,26 @@ return base::MD5String(url.spec()); } -base::FilePath OfflinePagePath(const GURL& url) { - base::FilePath directory(OfflineURLDirectoryID(url)); - return directory.Append(FILE_PATH_LITERAL(kMainPageFileName)); -} - base::FilePath OfflineURLDirectoryAbsolutePath( const base::FilePath& profile_path, const GURL& url) { - return OfflineRootDirectoryPath(profile_path) - .Append(OfflineURLDirectoryID(url)); + return OfflineURLAbsolutePathFromRelativePath( + profile_path, base::FilePath(OfflineURLDirectoryID(url))); } -base::FilePath OfflinePageAbsolutePath(const base::FilePath& profile_path, - const GURL& url) { - return OfflineRootDirectoryPath(profile_path).Append(OfflinePagePath(url)); +base::FilePath OfflinePagePath(const GURL& url, OfflineFileType type) { + base::FilePath directory(OfflineURLDirectoryID(url)); + switch (type) { + case OFFLINE_TYPE_HTML: + return directory.Append(FILE_PATH_LITERAL(kMainPageFileName)); + case OFFLINE_TYPE_PDF: + return directory.Append(FILE_PATH_LITERAL(kPDFFileName)); + } +} + +base::FilePath OfflineURLAbsolutePathFromRelativePath( + const base::FilePath& profile_path, + const base::FilePath& relative_path) { + return OfflineRootDirectoryPath(profile_path).Append(relative_path); } }
diff --git a/components/reading_list/ios/offline_url_utils.h b/components/reading_list/ios/offline_url_utils.h index 45af3d27..dc9060f 100644 --- a/components/reading_list/ios/offline_url_utils.h +++ b/components/reading_list/ios/offline_url_utils.h
@@ -12,11 +12,23 @@ namespace reading_list { +// The different types of file that can be stored for offline usage. +enum OfflineFileType { OFFLINE_TYPE_HTML = 0, OFFLINE_TYPE_PDF = 1 }; + // The absolute path of the directory where offline URLs are saved. // |profile_path| is the path to the profile directory that contain the offline // directory. base::FilePath OfflineRootDirectoryPath(const base::FilePath& profile_path); +// The absolute path of |relative_path| where |relative_path| is relative to the +// offline root folder (OfflineRootDirectoryPath). +// |profile_path| is the path to the profile directory that contain the offline +// directory. +// The file/directory may not exist. +base::FilePath OfflineURLAbsolutePathFromRelativePath( + const base::FilePath& profile_path, + const base::FilePath& relative_path); + // The absolute path of the directory where a |url| is saved offline. // Contains the page and supporting files (images). // |profile_path| is the path to the profile directory that contain the offline @@ -26,17 +38,11 @@ const base::FilePath& profile_path, const GURL& url); -// The absolute path of the offline webpage for the |url|. The file may not -// exist. -// |profile_path| is the path to the profile directory that contain the offline -// directory. -base::FilePath OfflinePageAbsolutePath(const base::FilePath& profile_path, - const GURL& url); - // The relative path to the offline webpage for the |url|. The result is -// relative to |OfflineRootDirectoryPath()|. +// relative to |OfflineRootDirectoryPath()|. |type| is the type of the file and +// will determine the extension of the returned value. // The file may not exist. -base::FilePath OfflinePagePath(const GURL& url); +base::FilePath OfflinePagePath(const GURL& url, OfflineFileType type); // The name of the directory containing offline data for |url|. std::string OfflineURLDirectoryID(const GURL& url);
diff --git a/components/reading_list/ios/offline_url_utils_unittest.cc b/components/reading_list/ios/offline_url_utils_unittest.cc index e2c56c1a..8d9199a 100644 --- a/components/reading_list/ios/offline_url_utils_unittest.cc +++ b/components/reading_list/ios/offline_url_utils_unittest.cc
@@ -38,20 +38,24 @@ offline_directory.value()); } -// Checks the offline page absolute path is -// |profile_path|/Offline/OfflineURLDirectoryID/page.html; -TEST(OfflineURLUtilsTest, OfflinePageAbsolutePathTest) { +// Checks the offline page directory is +// |profile_path|/Offline/OfflineURLDirectoryID; +TEST(OfflineURLUtilsTest, AbsolutePathForRelativePathTest) { base::FilePath profile_path("/profile_path"); - GURL url("http://www.google.com/test"); - base::FilePath offline_page = - reading_list::OfflinePageAbsolutePath(profile_path, url); - EXPECT_EQ("/profile_path/Offline/0090071ef710946a1263c276284bb3b8/page.html", - offline_page.value()); + base::FilePath relative_path("relative/path"); + base::FilePath absolute_path = + reading_list::OfflineURLAbsolutePathFromRelativePath(profile_path, + relative_path); + EXPECT_EQ("/profile_path/Offline/relative/path", absolute_path.value()); } // Checks the offline page path is OfflineURLDirectoryID/page.html; TEST(OfflineURLUtilsTest, OfflinePagePathTest) { GURL url("http://www.google.com/test"); - base::FilePath offline_page = reading_list::OfflinePagePath(url); + base::FilePath offline_page = + reading_list::OfflinePagePath(url, reading_list::OFFLINE_TYPE_HTML); EXPECT_EQ("0090071ef710946a1263c276284bb3b8/page.html", offline_page.value()); + offline_page = + reading_list::OfflinePagePath(url, reading_list::OFFLINE_TYPE_PDF); + EXPECT_EQ("0090071ef710946a1263c276284bb3b8/file.pdf", offline_page.value()); }
diff --git a/components/variations/android/BUILD.gn b/components/variations/android/BUILD.gn index 6ad4936..40e0b13 100644 --- a/components/variations/android/BUILD.gn +++ b/components/variations/android/BUILD.gn
@@ -12,7 +12,16 @@ java_files = [ "java/src/org/chromium/components/variations/VariationsAssociatedData.java", "java/src/org/chromium/components/variations/firstrun/VariationsSeedBridge.java", - "java/src/org/chromium/components/variations/firstrun/VariationsSeedService.java", - "java/src/org/chromium/components/variations/firstrun/VariationsSeedServiceLauncher.java", + "java/src/org/chromium/components/variations/firstrun/VariationsSeedFetcher.java", + ] +} + +junit_binary("components_variations_junit_tests") { + java_files = [ "junit/src/org/chromium/components/variations/firstrun/VariationsSeedFetcherTest.java" ] + deps = [ + ":variations_java", + "//base:base_java", + "//base:base_java_test_support", + "//third_party/hamcrest:hamcrest_java", ] }
diff --git a/components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedService.java b/components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedFetcher.java similarity index 64% rename from components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedService.java rename to components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedFetcher.java index 2e57753..7f117590 100644 --- a/components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedService.java +++ b/components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedFetcher.java
@@ -4,12 +4,14 @@ package org.chromium.components.variations.firstrun; -import android.app.IntentService; -import android.content.Intent; +import android.content.Context; +import android.content.SharedPreferences; import android.os.SystemClock; -import android.support.v4.content.LocalBroadcastManager; +import org.chromium.base.ContextUtils; import org.chromium.base.Log; +import org.chromium.base.ThreadUtils; +import org.chromium.base.VisibleForTesting; import org.chromium.base.metrics.CachedMetrics.SparseHistogramSample; import org.chromium.base.metrics.CachedMetrics.TimesHistogramSample; @@ -17,21 +19,20 @@ import java.io.IOException; import java.io.InputStream; import java.net.HttpURLConnection; +import java.net.MalformedURLException; import java.net.SocketTimeoutException; import java.net.URL; import java.net.UnknownHostException; import java.util.concurrent.TimeUnit; /** - * Background service that fetches the variations seed before the actual first run of Chrome. + * Fetches the variations seed before the actual first run of Chrome. */ -public class VariationsSeedService extends IntentService { - private static final String TAG = "VariationsSeedServ"; - - public static final String COMPLETE_BROADCAST = "VariationsseedService.Complete"; - +public class VariationsSeedFetcher { + private static final String TAG = "VariationsSeedFetch"; private static final String VARIATIONS_SERVER_URL = "https://clientservices.googleapis.com/chrome-variations/seed?osname=android"; + private static final int BUFFER_SIZE = 4096; private static final int READ_TIMEOUT = 3000; // time in ms private static final int REQUEST_TIMEOUT = 1000; // time in ms @@ -43,31 +44,65 @@ private static final int SEED_FETCH_RESULT_TIMEOUT = -2; private static final int SEED_FETCH_RESULT_IOEXCEPTION = -1; - public VariationsSeedService() { - super(TAG); - } + @VisibleForTesting + static final String VARIATIONS_INITIALIZED_PREF = "variations_initialized"; - @Override - public void onHandleIntent(Intent intent) { - // Early return if the seed has already been fetched. In such a case, either the Java-side - // variations seed pref is set, or a different Java pref is set that indicates that the - // seed exists in the native prefs. - // Note: There is no need to check for a concurrent seed fetch here, because the service - // runs all its intents on the same worker thread serially. - if (VariationsSeedBridge.hasJavaPref(getApplicationContext()) - || VariationsSeedBridge.hasNativePref(getApplicationContext())) { - broadcastCompleteIntent(); - return; - } - try { - downloadContent(); - } finally { - broadcastCompleteIntent(); + // Synchronization lock + private static final Object sLock = new Object(); + + private static VariationsSeedFetcher sInstance; + + @VisibleForTesting + VariationsSeedFetcher() {} + + public static VariationsSeedFetcher get() { + // TODO(aberent) Check not running on UI thread. Doing so however makes Robolectric testing + // of dependent classes difficult. + synchronized (sLock) { + if (sInstance == null) { + sInstance = new VariationsSeedFetcher(); + } + return sInstance; } } - private void broadcastCompleteIntent() { - LocalBroadcastManager.getInstance(this).sendBroadcast(new Intent(COMPLETE_BROADCAST)); + /** + * Override the VariationsSeedFetcher, typically with a mock, for testing classes that depend on + * this one. + * @param fetcher the mock. + */ + @VisibleForTesting + public static void setVariationsSeedFetcherForTesting(VariationsSeedFetcher fetcher) { + sInstance = fetcher; + } + + @VisibleForTesting + protected HttpURLConnection getServerConnection() throws MalformedURLException, IOException { + URL url = new URL(VARIATIONS_SERVER_URL); + return (HttpURLConnection) url.openConnection(); + } + + /** + * Fetch the first run variations seed. + */ + public void fetchSeed() { + assert !ThreadUtils.runningOnUiThread(); + // Prevent multiple simultaneous fetches + synchronized (sLock) { + Context context = ContextUtils.getApplicationContext(); + SharedPreferences prefs = ContextUtils.getAppSharedPreferences(); + // Early return if an attempt has already been made to fetch the seed, even if it + // failed. Only attempt to get the initial Java seed once, since a failure probably + // indicates a network problem that is unlikely to be resolved by a second attempt. + // Note that VariationsSeedBridge.hasNativePref() is a pure Java function, reading an + // Android preference that is set when the seed is fetched by the native code. + if (prefs.getBoolean(VARIATIONS_INITIALIZED_PREF, false) + || VariationsSeedBridge.hasNativePref(context)) { + return; + } + downloadContent(context); + prefs.edit().putBoolean(VARIATIONS_INITIALIZED_PREF, true).apply(); + } } private void recordFetchResultOrCode(int resultOrCode) { @@ -89,12 +124,11 @@ histogram.record(timeDeltaMillis); } - private boolean downloadContent() { + private void downloadContent(Context context) { HttpURLConnection connection = null; try { long startTimeMillis = SystemClock.elapsedRealtime(); - URL url = new URL(VARIATIONS_SERVER_URL); - connection = (HttpURLConnection) url.openConnection(); + connection = getServerConnection(); connection.setReadTimeout(READ_TIMEOUT); connection.setConnectTimeout(REQUEST_TIMEOUT); connection.setDoInput(true); @@ -104,7 +138,7 @@ recordFetchResultOrCode(responseCode); if (responseCode != HttpURLConnection.HTTP_OK) { Log.w(TAG, "Non-OK response code = %d", responseCode); - return false; + return; } recordSeedConnectTime(SystemClock.elapsedRealtime() - startTimeMillis); @@ -115,21 +149,17 @@ String date = getHeaderFieldOrEmpty(connection, "Date"); boolean isGzipCompressed = getHeaderFieldOrEmpty(connection, "IM").equals("gzip"); VariationsSeedBridge.setVariationsFirstRunSeed( - getApplicationContext(), rawSeed, signature, country, date, isGzipCompressed); + context, rawSeed, signature, country, date, isGzipCompressed); recordSeedFetchTime(SystemClock.elapsedRealtime() - startTimeMillis); - return true; } catch (SocketTimeoutException e) { recordFetchResultOrCode(SEED_FETCH_RESULT_TIMEOUT); Log.w(TAG, "SocketTimeoutException fetching first run seed: ", e); - return false; } catch (UnknownHostException e) { recordFetchResultOrCode(SEED_FETCH_RESULT_UNKNOWN_HOST_EXCEPTION); Log.w(TAG, "UnknownHostException fetching first run seed: ", e); - return false; } catch (IOException e) { recordFetchResultOrCode(SEED_FETCH_RESULT_IOEXCEPTION); Log.w(TAG, "IOException fetching first run seed: ", e); - return false; } finally { if (connection != null) { connection.disconnect();
diff --git a/components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedServiceLauncher.java b/components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedServiceLauncher.java deleted file mode 100644 index 69f04c8a..0000000 --- a/components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedServiceLauncher.java +++ /dev/null
@@ -1,27 +0,0 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -package org.chromium.components.variations.firstrun; - -import android.content.BroadcastReceiver; -import android.content.Context; -import android.content.Intent; - -/** - * This receiver is notified when a user goes through the Setup Wizard and acknowledges - * the Chrome ToS and after that we start a variations service to fetch variations seed - * before the actual first Chrome launch. - * - * TODO(agulenko): Implement working with another broadcast (e.g. connectivity change). - */ -public class VariationsSeedServiceLauncher extends BroadcastReceiver { - private static final String TAG = "VariationsSeedServiceLauncher"; - - @Override - public void onReceive(Context context, Intent intent) { - // Start of service to fetch variations seed from the server to use it on Chrome first run - Intent serviceIntent = new Intent(context, VariationsSeedService.class); - context.startService(serviceIntent); - } -} \ No newline at end of file
diff --git a/components/variations/android/junit/src/org/chromium/components/variations/firstrun/VariationsSeedFetcherTest.java b/components/variations/android/junit/src/org/chromium/components/variations/firstrun/VariationsSeedFetcherTest.java new file mode 100644 index 0000000..9c72510 --- /dev/null +++ b/components/variations/android/junit/src/org/chromium/components/variations/firstrun/VariationsSeedFetcherTest.java
@@ -0,0 +1,132 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.components.variations.firstrun; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.SharedPreferences; +import android.os.Looper; +import android.util.Base64; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.annotation.Config; + +import org.chromium.base.ContextUtils; +import org.chromium.base.ThreadUtils; +import org.chromium.testing.local.LocalRobolectricTestRunner; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.net.HttpURLConnection; + +/** + * Tests for VariationsSeedFetcher + */ +@RunWith(LocalRobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class VariationsSeedFetcherTest { + private HttpURLConnection mConnection; + private VariationsSeedFetcher mFetcher; + private SharedPreferences mPrefs; + + @Before + public void setUp() throws IOException { + // Pretend we are not on the UI thread, since the class we are testing is supposed to run + // only on a background thread. + ThreadUtils.setUiThread(mock(Looper.class)); + mFetcher = spy(VariationsSeedFetcher.get()); + mConnection = mock(HttpURLConnection.class); + doReturn(mConnection).when(mFetcher).getServerConnection(); + mPrefs = ContextUtils.getAppSharedPreferences(); + mPrefs.edit().clear().apply(); + } + + @After + public void tearDown() { + ThreadUtils.setUiThread(null); + } + + /** + * Test method for {@link VariationsSeedFetcher#fetchSeed()}. + * + * @throws IOException + */ + @Test + public void testFetchSeed() throws IOException { + // Pretend we are on a background thread; set the UI thread looper to something other than + // the current thread. + + when(mConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK); + when(mConnection.getHeaderField("X-Seed-Signature")).thenReturn("signature"); + when(mConnection.getHeaderField("X-Country")).thenReturn("Nowhere Land"); + when(mConnection.getHeaderField("Date")).thenReturn("A date"); + when(mConnection.getHeaderField("IM")).thenReturn("gzip"); + when(mConnection.getInputStream()).thenReturn(new ByteArrayInputStream("1234".getBytes())); + + mFetcher.fetchSeed(); + + assertThat(mPrefs.getString(VariationsSeedBridge.VARIATIONS_FIRST_RUN_SEED_SIGNATURE, ""), + equalTo("signature")); + assertThat(mPrefs.getString(VariationsSeedBridge.VARIATIONS_FIRST_RUN_SEED_COUNTRY, ""), + equalTo("Nowhere Land")); + assertThat(mPrefs.getString(VariationsSeedBridge.VARIATIONS_FIRST_RUN_SEED_DATE, ""), + equalTo("A date")); + assertTrue(mPrefs.getBoolean( + VariationsSeedBridge.VARIATIONS_FIRST_RUN_SEED_IS_GZIP_COMPRESSED, false)); + assertThat(mPrefs.getString(VariationsSeedBridge.VARIATIONS_FIRST_RUN_SEED_BASE64, ""), + equalTo(Base64.encodeToString("1234".getBytes(), Base64.NO_WRAP))); + } + + /** + * Test method for {@link VariationsSeedFetcher#fetchSeed()} when no fetch is needed + */ + @Test + public void testFetchSeed_noFetchNeeded() throws IOException { + mPrefs.edit().putBoolean(VariationsSeedFetcher.VARIATIONS_INITIALIZED_PREF, true).apply(); + + mFetcher.fetchSeed(); + + verify(mConnection, never()).connect(); + } + + /** + * Test method for {@link VariationsSeedFetcher#fetchSeed()} with a bad response + */ + @Test + public void testFetchSeed_badResponse() throws IOException { + when(mConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_NOT_FOUND); + + mFetcher.fetchSeed(); + + assertTrue(mPrefs.getBoolean(VariationsSeedFetcher.VARIATIONS_INITIALIZED_PREF, false)); + assertFalse(VariationsSeedBridge.hasJavaPref(ContextUtils.getApplicationContext())); + } + + /** + * Test method for {@link VariationsSeedFetcher#fetchSeed()} with an exception when connecting + */ + @Test + public void testFetchSeed_IOException() throws IOException { + doThrow(new IOException()).when(mConnection).connect(); + + mFetcher.fetchSeed(); + + assertTrue(mPrefs.getBoolean(VariationsSeedFetcher.VARIATIONS_INITIALIZED_PREF, false)); + assertFalse(VariationsSeedBridge.hasJavaPref(ContextUtils.getApplicationContext())); + } +}
diff --git a/device/generic_sensor/generic_sensor_consts.h b/device/generic_sensor/generic_sensor_consts.h index 63e3e18..ca9b45a4 100644 --- a/device/generic_sensor/generic_sensor_consts.h +++ b/device/generic_sensor/generic_sensor_consts.h
@@ -13,8 +13,8 @@ // Required for conversion from G/s^2 to m/s^2 constexpr double kMeanGravity = 9.80665; -// Required for conversion from deg/s^2 to rad/s^2 -constexpr double kRadiansInDegreesPerSecond = M_PI / 180.0; +// Required for conversion from deg to rad +constexpr double kRadiansInDegrees = M_PI / 180.0; // Required for conversion from Gauss to uT. constexpr double kMicroteslaInGauss = 100.0;
diff --git a/device/generic_sensor/linux/sensor_data_linux.cc b/device/generic_sensor/linux/sensor_data_linux.cc index f4ee366..8e506a9 100644 --- a/device/generic_sensor/linux/sensor_data_linux.cc +++ b/device/generic_sensor/linux/sensor_data_linux.cc
@@ -111,14 +111,14 @@ #if defined(OS_CHROMEOS) data->sensor_scale_name = "in_anglvel_base_scale"; data->sensor_frequency_file_name = "in_anglvel_base_frequency"; - data->apply_scaling_func = base::Bind([](double scaling_value, double offset, - SensorReading& reading) { - double scaling = kMeanGravity * kRadiansInDegreesPerSecond / scaling_value; - // Adapt CrOS reading values to generic sensor api specs. - reading.values[0] = -scaling * (reading.values[0] + offset); - reading.values[1] = -scaling * (reading.values[1] + offset); - reading.values[2] = -scaling * (reading.values[2] + offset); - }); + data->apply_scaling_func = base::Bind( + [](double scaling_value, double offset, SensorReading& reading) { + double scaling = kMeanGravity * kRadiansInDegrees / scaling_value; + // Adapt CrOS reading values to generic sensor api specs. + reading.values[0] = -scaling * (reading.values[0] + offset); + reading.values[1] = -scaling * (reading.values[1] + offset); + reading.values[2] = -scaling * (reading.values[2] + offset); + }); #else data->sensor_scale_name = "in_anglvel_scale"; data->sensor_offset_file_name = "in_anglvel_offset";
diff --git a/device/generic_sensor/platform_sensor_and_provider_unittest_linux.cc b/device/generic_sensor/platform_sensor_and_provider_unittest_linux.cc index 8cfc567..907d12c 100644 --- a/device/generic_sensor/platform_sensor_and_provider_unittest_linux.cc +++ b/device/generic_sensor/platform_sensor_and_provider_unittest_linux.cc
@@ -593,8 +593,7 @@ SensorReadingSharedBuffer* buffer = static_cast<SensorReadingSharedBuffer*>(mapping.get()); #if defined(OS_CHROMEOS) - double scaling = - kMeanGravity * kRadiansInDegreesPerSecond / kGyroscopeScalingValue; + double scaling = kMeanGravity * kRadiansInDegrees / kGyroscopeScalingValue; EXPECT_THAT(buffer->reading.values[0], -scaling * sensor_values[0]); EXPECT_THAT(buffer->reading.values[1], -scaling * sensor_values[1]); EXPECT_THAT(buffer->reading.values[2], -scaling * sensor_values[2]);
diff --git a/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc b/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc index 6b8b0f1..8f3eb36 100644 --- a/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc +++ b/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc
@@ -602,22 +602,16 @@ double z_ang_accel = -98.7; GenerateDataUpdatedEvent( - {{SENSOR_DATA_TYPE_ANGULAR_ACCELERATION_X_DEGREES_PER_SECOND_SQUARED, - x_ang_accel}, - {SENSOR_DATA_TYPE_ANGULAR_ACCELERATION_Y_DEGREES_PER_SECOND_SQUARED, - y_ang_accel}, - {SENSOR_DATA_TYPE_ANGULAR_ACCELERATION_Z_DEGREES_PER_SECOND_SQUARED, - z_ang_accel}}); + {{SENSOR_DATA_TYPE_ANGULAR_VELOCITY_X_DEGREES_PER_SECOND, x_ang_accel}, + {SENSOR_DATA_TYPE_ANGULAR_VELOCITY_Y_DEGREES_PER_SECOND, y_ang_accel}, + {SENSOR_DATA_TYPE_ANGULAR_VELOCITY_Z_DEGREES_PER_SECOND, z_ang_accel}}); base::RunLoop().RunUntilIdle(); SensorReadingSharedBuffer* buffer = static_cast<SensorReadingSharedBuffer*>(mapping.get()); - EXPECT_THAT(buffer->reading.values[0], - -x_ang_accel * kRadiansInDegreesPerSecond); - EXPECT_THAT(buffer->reading.values[1], - -y_ang_accel * kRadiansInDegreesPerSecond); - EXPECT_THAT(buffer->reading.values[2], - -z_ang_accel * kRadiansInDegreesPerSecond); + EXPECT_THAT(buffer->reading.values[0], -x_ang_accel * kRadiansInDegrees); + EXPECT_THAT(buffer->reading.values[1], -y_ang_accel * kRadiansInDegrees); + EXPECT_THAT(buffer->reading.values[2], -z_ang_accel * kRadiansInDegrees); EXPECT_TRUE(sensor->StopListening(client.get(), configuration)); }
diff --git a/device/generic_sensor/platform_sensor_reader_win.cc b/device/generic_sensor/platform_sensor_reader_win.cc index d19b61f..5b77b6c8 100644 --- a/device/generic_sensor/platform_sensor_reader_win.cc +++ b/device/generic_sensor/platform_sensor_reader_win.cc
@@ -109,23 +109,23 @@ double y = 0.0; double z = 0.0; if (!GetReadingValueForProperty( - SENSOR_DATA_TYPE_ANGULAR_ACCELERATION_X_DEGREES_PER_SECOND_SQUARED, - report, &x) || + SENSOR_DATA_TYPE_ANGULAR_VELOCITY_X_DEGREES_PER_SECOND, report, + &x) || !GetReadingValueForProperty( - SENSOR_DATA_TYPE_ANGULAR_ACCELERATION_Y_DEGREES_PER_SECOND_SQUARED, - report, &y) || + SENSOR_DATA_TYPE_ANGULAR_VELOCITY_Y_DEGREES_PER_SECOND, report, + &y) || !GetReadingValueForProperty( - SENSOR_DATA_TYPE_ANGULAR_ACCELERATION_Z_DEGREES_PER_SECOND_SQUARED, - report, &z)) { + SENSOR_DATA_TYPE_ANGULAR_VELOCITY_Z_DEGREES_PER_SECOND, report, + &z)) { return E_FAIL; } // Windows uses coordinate system where Z axis points down from device // screen, therefore, using right hand notation, we have to reverse - // sign for each axis. Values are converted from deg/s^2 to rad/s^2. - reading.values[0] = -x * kRadiansInDegreesPerSecond; - reading.values[1] = -y * kRadiansInDegreesPerSecond; - reading.values[2] = -z * kRadiansInDegreesPerSecond; + // sign for each axis. Values are converted from deg to rad. + reading.values[0] = -x * kRadiansInDegrees; + reading.values[1] = -y * kRadiansInDegrees; + reading.values[2] = -z * kRadiansInDegrees; return S_OK; }); return params;
diff --git a/ios/chrome/app/strings/ios_strings.grd b/ios/chrome/app/strings/ios_strings.grd index 0ca1db6..71f9322 100644 --- a/ios/chrome/app/strings/ios_strings.grd +++ b/ios/chrome/app/strings/ios_strings.grd
@@ -1305,6 +1305,9 @@ <message name="IDS_IOS_SPOTLIGHT_KEYWORD_TWO" desc="Keyword used in Spotlight that will be linked to the Chrome App. [iOS only]"> Internet </message> + <message name="IDS_IOS_SUGGESTIONS_DONE" desc="Label of the navigation bar button to close the view."> + Done + </message> <message name="IDS_IOS_SWITCH_BROWSER_MODE_ENTER_INCOGNITO" desc="The accessibility label of the switch browser mode button to enter the Incognito mode [iOS only]"> Enter Incognito Mode </message> @@ -1522,6 +1525,9 @@ <message name="IDS_IOS_TOOLS_MENU_SHARE" desc="The iOS menu item for sharing the current URL [Length: 15em] [iOS only]"> Share... </message> + <message name="IDS_IOS_TOOLS_MENU_SUGGESTIONS" desc="The iOS menu item for displaying the Suggestions UI [Length: 15em] [iOS only]"> + Suggestions + </message> <message name="IDS_IOS_TRANSLATE_SETTING" desc="Title for the view and option in Settings for Translate. [Length: 25em] [iOS only]"> Google Translate </message>
diff --git a/ios/chrome/browser/about_flags.mm b/ios/chrome/browser/about_flags.mm index a681f344..747de62 100644 --- a/ios/chrome/browser/about_flags.mm +++ b/ios/chrome/browser/about_flags.mm
@@ -258,6 +258,14 @@ command_line->AppendSwitch(switches::kDisableDownloadImageRenaming); } + // Populate command line flag for Suggestions UI display. + NSString* enableSuggestions = [defaults stringForKey:@"EnableSuggestions"]; + if ([enableSuggestions isEqualToString:@"Enabled"]) { + command_line->AppendSwitch(switches::kEnableSuggestionsUI); + } else if ([enableSuggestions isEqualToString:@"Disabled"]) { + command_line->AppendSwitch(switches::kDisableSuggestionsUI); + } + // Freeform commandline flags. These are added last, so that any flags added // earlier in this function take precedence. if ([defaults boolForKey:@"EnableFreeformCommandLineFlags"]) {
diff --git a/ios/chrome/browser/chrome_switches.cc b/ios/chrome/browser/chrome_switches.cc index 2d15912c..d4495b4e 100644 --- a/ios/chrome/browser/chrome_switches.cc +++ b/ios/chrome/browser/chrome_switches.cc
@@ -59,9 +59,12 @@ // Disables Physical Web scanning for nearby URLs. const char kDisableIOSPhysicalWeb[] = "disable-ios-physical-web"; -// Enables the string change from "Save Image" to "Download Image". +// Disables the string change from "Save Image" to "Download Image". const char kDisableDownloadImageRenaming[] = "disable-download-image-renaming"; +// Disables the Suggestions UI +const char kDisableSuggestionsUI[] = "disable-suggestions-ui"; + // Enables all bookmarks view in bookmark manager. const char kEnableAllBookmarksView[] = "enable-all-bookmarks-view"; @@ -114,6 +117,9 @@ // Enables the string change from "Save Image" to "Download Image". const char kEnableDownloadImageRenaming[] = "enable-download-image-renaming"; +// Enables the Suggestions UI +const char kEnableSuggestionsUI[] = "enable-suggestions-ui"; + // Forces additional Chrome Variation Ids that will be sent in X-Client-Data // header, specified as a 64-bit encoded list of numeric experiment ids. Ids // prefixed with the character "t" will be treated as Trigger Variation Ids.
diff --git a/ios/chrome/browser/chrome_switches.h b/ios/chrome/browser/chrome_switches.h index cf5b65a..51ccb5a 100644 --- a/ios/chrome/browser/chrome_switches.h +++ b/ios/chrome/browser/chrome_switches.h
@@ -25,6 +25,7 @@ extern const char kDisableTabSwitcher[]; extern const char kDisableIOSPhysicalWeb[]; extern const char kDisableDownloadImageRenaming[]; +extern const char kDisableSuggestionsUI[]; extern const char kEnableAllBookmarksView[]; extern const char kEnableContextualSearch[]; @@ -43,6 +44,7 @@ extern const char kEnableTabSwitcher[]; extern const char kEnableIOSPhysicalWeb[]; extern const char kEnableDownloadImageRenaming[]; +extern const char kEnableSuggestionsUI[]; extern const char kIOSForceVariationIds[]; extern const char kIOSMetricsRecordingOnly[];
diff --git a/ios/chrome/browser/content_suggestions/BUILD.gn b/ios/chrome/browser/content_suggestions/BUILD.gn new file mode 100644 index 0000000..8a5a3d5 --- /dev/null +++ b/ios/chrome/browser/content_suggestions/BUILD.gn
@@ -0,0 +1,18 @@ +# Copyright 2017 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +source_set("content_suggestions") { + configs += [ "//build/config/compiler:enable_arc" ] + sources = [ + "content_suggestions_coordinator.h", + "content_suggestions_coordinator.mm", + ] + deps = [ + "//base", + "//ios/chrome/app/strings", + "//ios/chrome/browser", + "//ios/chrome/browser/ui/content_suggestions", + "//ui/base", + ] +}
diff --git a/ios/chrome/browser/ui/suggestions/OWNERS b/ios/chrome/browser/content_suggestions/OWNERS similarity index 100% copy from ios/chrome/browser/ui/suggestions/OWNERS copy to ios/chrome/browser/content_suggestions/OWNERS
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.h b/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.h new file mode 100644 index 0000000..8c051bb --- /dev/null +++ b/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.h
@@ -0,0 +1,19 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COORDINATOR_H_ +#define IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COORDINATOR_H_ + +#import "ios/chrome/browser/chrome_coordinator.h" + +// Coordinator to manage the Suggestions UI via a +// ContentSuggestionsViewController. +@interface ContentSuggestionsCoordinator : ChromeCoordinator + +// Whether the Suggestions UI is displayed. If this is true, start is a no-op. +@property(nonatomic, readonly) BOOL visible; + +@end + +#endif // IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COORDINATOR_H_
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm b/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm new file mode 100644 index 0000000..0df1af28 --- /dev/null +++ b/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm
@@ -0,0 +1,77 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import "ios/chrome/browser/content_suggestions/content_suggestions_coordinator.h" + +#include "base/mac/scoped_nsobject.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_commands.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h" +#include "ios/chrome/grit/ios_strings.h" +#include "ui/base/l10n/l10n_util.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +@interface ContentSuggestionsCoordinator ()<ContentSuggestionsCommands> { + UINavigationController* _navigationController; +} + +@end + +@implementation ContentSuggestionsCoordinator + +@synthesize visible = _visible; + +- (void)start { + if (self.visible) { + // Prevent this coordinator from being started twice in a row. + return; + } + + _visible = YES; + + ContentSuggestionsViewController* suggestionsViewController = + [[ContentSuggestionsViewController alloc] + initWithStyle:CollectionViewControllerStyleDefault]; + + suggestionsViewController.suggestionCommandHandler = self; + _navigationController = [[UINavigationController alloc] + initWithRootViewController:suggestionsViewController]; + + suggestionsViewController.navigationItem.leftBarButtonItem = + [[UIBarButtonItem alloc] + initWithTitle:l10n_util::GetNSString(IDS_IOS_SUGGESTIONS_DONE) + style:UIBarButtonItemStylePlain + target:self + action:@selector(stop)]; + + [self.baseViewController presentViewController:_navigationController + animated:YES + completion:nil]; +} + +- (void)stop { + [[_navigationController presentingViewController] + dismissViewControllerAnimated:YES + completion:nil]; + _navigationController = nil; + _visible = NO; +} + +#pragma mark - ContentSuggestionsCommands + +- (void)openReadingList { +} + +- (void)openFirstPageOfReadingList { +} + +- (void)addEmptyItem { +} + +- (void)openFaviconAtIndex:(NSInteger)index { +} + +@end
diff --git a/ios/chrome/browser/experimental_flags.h b/ios/chrome/browser/experimental_flags.h index 4dfd356..e53dec8 100644 --- a/ios/chrome/browser/experimental_flags.h +++ b/ios/chrome/browser/experimental_flags.h
@@ -119,6 +119,9 @@ // only. bool UseOnlyLocalHeuristicsForPasswordGeneration(); +// Whether the Suggestions UI is enabled. +bool IsSuggestionsUIEnabled(); + } // namespace experimental_flags #endif // IOS_CHROME_BROWSER_EXPERIMENTAL_FLAGS_H_
diff --git a/ios/chrome/browser/experimental_flags.mm b/ios/chrome/browser/experimental_flags.mm index d4d26e7..31fc043 100644 --- a/ios/chrome/browser/experimental_flags.mm +++ b/ios/chrome/browser/experimental_flags.mm
@@ -309,4 +309,17 @@ autofill::switches::kLocalHeuristicsOnlyForPasswordGeneration); } +bool IsSuggestionsUIEnabled() { + // Check if the experimental flag is forced on or off. + base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); + if (command_line->HasSwitch(switches::kEnableSuggestionsUI)) + return true; + + if (command_line->HasSwitch(switches::kDisableSuggestionsUI)) + return false; + + // By default, disable it. + return false; +} + } // namespace experimental_flags
diff --git a/ios/chrome/browser/reading_list/reading_list_distiller_page.h b/ios/chrome/browser/reading_list/reading_list_distiller_page.h index 44d29ca..eae2b3af 100644 --- a/ios/chrome/browser/reading_list/reading_list_distiller_page.h +++ b/ios/chrome/browser/reading_list/reading_list_distiller_page.h
@@ -20,21 +20,40 @@ class FaviconWebStateDispatcher; +// A delegate to get information on the page that is actually loaded during +// distillation. +class ReadingListDistillerPageDelegate { + public: + virtual ~ReadingListDistillerPageDelegate(); + + // A callback called if the URL passed to the distilled led to a redirection. + virtual void DistilledPageRedirectedToURL(const GURL& original_url, + const GURL& final_url) = 0; + // Gives to the delegate the mime type of the loaded page. + // If the type is not "text/html", the distillation will fail. + virtual void DistilledPageHasMimeType(const GURL& original_url, + const std::string& mime_type) = 0; + + protected: + ReadingListDistillerPageDelegate(); + DISALLOW_COPY_AND_ASSIGN(ReadingListDistillerPageDelegate); +}; + // An DistillerPageIOS that will retain WebState to allow favicon download and // and add a 2 seconds delay between loading and distillation. class ReadingListDistillerPage : public dom_distiller::DistillerPageIOS { public: typedef base::Callback<void(const GURL&, const GURL&)> RedirectionCallback; + // Creates a ReadingListDistillerPage. WebStates to download the pages will + // be provided by web_state_dispatcher. + // |browser_state|, |web_state_dispatcher| and |delegate| must not be null. explicit ReadingListDistillerPage( web::BrowserState* browser_state, - FaviconWebStateDispatcher* web_state_dispatcher); + FaviconWebStateDispatcher* web_state_dispatcher, + ReadingListDistillerPageDelegate* delegate); ~ReadingListDistillerPage() override; - // Sets a callback that will be called just before distillation happen with - // the URL of the page that will effectively be distilled. - void SetRedirectionCallback(RedirectionCallback redirection_callback); - protected: void DistillPageImpl(const GURL& url, const std::string& script) override; void OnDistillationDone(const GURL& page_url, @@ -66,12 +85,13 @@ // Continues distillation by calling superclass |OnLoadURLDone|. void DelayedOnLoadURLDone(); - - RedirectionCallback redirection_callback_; GURL original_url_; FaviconWebStateDispatcher* web_state_dispatcher_; + ReadingListDistillerPageDelegate* delegate_; base::WeakPtrFactory<ReadingListDistillerPage> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(ReadingListDistillerPage); }; } // namespace reading_list
diff --git a/ios/chrome/browser/reading_list/reading_list_distiller_page.mm b/ios/chrome/browser/reading_list/reading_list_distiller_page.mm index 8a3d58c7..baa40124 100644 --- a/ios/chrome/browser/reading_list/reading_list_distiller_page.mm +++ b/ios/chrome/browser/reading_list/reading_list_distiller_page.mm
@@ -30,20 +30,22 @@ namespace reading_list { +ReadingListDistillerPageDelegate::ReadingListDistillerPageDelegate() {} +ReadingListDistillerPageDelegate::~ReadingListDistillerPageDelegate() {} + ReadingListDistillerPage::ReadingListDistillerPage( web::BrowserState* browser_state, - FaviconWebStateDispatcher* web_state_dispatcher) + FaviconWebStateDispatcher* web_state_dispatcher, + ReadingListDistillerPageDelegate* delegate) : dom_distiller::DistillerPageIOS(browser_state), web_state_dispatcher_(web_state_dispatcher), - weak_ptr_factory_(this) {} + delegate_(delegate), + weak_ptr_factory_(this) { + DCHECK(delegate); +} ReadingListDistillerPage::~ReadingListDistillerPage() {} -void ReadingListDistillerPage::SetRedirectionCallback( - RedirectionCallback redirection_callback) { - redirection_callback_ = redirection_callback; -} - void ReadingListDistillerPage::DistillPageImpl(const GURL& url, const std::string& script) { std::unique_ptr<web::WebState> old_web_state = DetachWebState(); @@ -106,6 +108,15 @@ DistillerPageIOS::OnLoadURLDone(load_completion_status); return; } + delegate_->DistilledPageHasMimeType(original_url_, + CurrentWebState()->GetContentsMimeType()); + if (!CurrentWebState()->ContentIsHTML()) { + // If content is not HTML, distillation will fail immediatly. + // Call the handler to make sure cleaning methods are called correctly. + // There is no need to wait for rendering either. + DistillerPageIOS::OnLoadURLDone(load_completion_status); + return; + } // Page is loaded but rendering may not be done yet. Give a delay to the page. base::WeakPtr<ReadingListDistillerPage> weak_this = weak_ptr_factory_.GetWeakPtr(); @@ -129,8 +140,8 @@ // If the visible URL is not the original URL, notify the caller that URL // changed. GURL redirected_url = CurrentWebState()->GetVisibleURL(); - if (redirected_url != original_url_ && !redirection_callback_.is_null()) { - redirection_callback_.Run(original_url_, redirected_url); + if (redirected_url != original_url_ && delegate_) { + delegate_->DistilledPageRedirectedToURL(original_url_, redirected_url); } DistillerPageIOS::OnLoadURLDone(web::PageLoadCompletionStatus::SUCCESS); }
diff --git a/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h b/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h index 052cac64..71acfca 100644 --- a/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h +++ b/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h
@@ -7,35 +7,30 @@ #include <memory> -#include "components/dom_distiller/core/distiller_page.h" +#include "base/macros.h" namespace web { class BrowserState; } namespace reading_list { + class FaviconWebStateDispatcher; class ReadingListDistillerPage; +class ReadingListDistillerPageDelegate; // ReadingListDistillerPageFactory is an iOS-specific implementation of the // DistillerPageFactory interface allowing the creation of DistillerPage // instances. // These instances are configured to distille the articles of the Reading List. -class ReadingListDistillerPageFactory - : public dom_distiller::DistillerPageFactory { +class ReadingListDistillerPageFactory { public: explicit ReadingListDistillerPageFactory(web::BrowserState* browser_state); - ~ReadingListDistillerPageFactory() override; + virtual ~ReadingListDistillerPageFactory(); // Creates a ReadingListDistillerPage. - std::unique_ptr<reading_list::ReadingListDistillerPage> - CreateReadingListDistillerPage() const; - - // Implementation of DistillerPageFactory: - std::unique_ptr<dom_distiller::DistillerPage> CreateDistillerPage( - const gfx::Size& view_size) const override; - std::unique_ptr<dom_distiller::DistillerPage> CreateDistillerPageWithHandle( - std::unique_ptr<dom_distiller::SourcePageHandle> handle) const override; + std::unique_ptr<ReadingListDistillerPage> CreateReadingListDistillerPage( + ReadingListDistillerPageDelegate* delegate) const; private: web::BrowserState* browser_state_;
diff --git a/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.mm b/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.mm index 96572e2..9aa7401 100644 --- a/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.mm +++ b/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.mm
@@ -21,24 +21,11 @@ ReadingListDistillerPageFactory::~ReadingListDistillerPageFactory() {} -std::unique_ptr<reading_list::ReadingListDistillerPage> -ReadingListDistillerPageFactory::CreateReadingListDistillerPage() const { +std::unique_ptr<ReadingListDistillerPage> +ReadingListDistillerPageFactory::CreateReadingListDistillerPage( + ReadingListDistillerPageDelegate* delegate) const { return base::MakeUnique<ReadingListDistillerPage>( - browser_state_, web_state_dispatcher_.get()); -} - -std::unique_ptr<dom_distiller::DistillerPage> -ReadingListDistillerPageFactory::CreateDistillerPage( - const gfx::Size& view_size) const { - return base::MakeUnique<ReadingListDistillerPage>( - browser_state_, web_state_dispatcher_.get()); -} - -std::unique_ptr<dom_distiller::DistillerPage> -ReadingListDistillerPageFactory::CreateDistillerPageWithHandle( - std::unique_ptr<dom_distiller::SourcePageHandle> handle) const { - return base::MakeUnique<ReadingListDistillerPage>( - browser_state_, web_state_dispatcher_.get()); + browser_state_, web_state_dispatcher_.get(), delegate); } } // namespace reading_list
diff --git a/ios/chrome/browser/reading_list/reading_list_download_service.cc b/ios/chrome/browser/reading_list/reading_list_download_service.cc index daab65c..6cc7c6c 100644 --- a/ios/chrome/browser/reading_list/reading_list_download_service.cc +++ b/ios/chrome/browser/reading_list/reading_list_download_service.cc
@@ -43,6 +43,7 @@ dom_distiller::DomDistillerService* distiller_service, PrefService* prefs, base::FilePath chrome_profile_path, + net::URLRequestContextGetter* url_request_context_getter, std::unique_ptr<reading_list::ReadingListDistillerPageFactory> distiller_page_factory) : reading_list_model_(reading_list_model), @@ -54,7 +55,7 @@ url_downloader_ = base::MakeUnique<URLDownloader>( distiller_service, distiller_page_factory_.get(), prefs, - chrome_profile_path, + chrome_profile_path, url_request_context_getter, base::Bind(&ReadingListDownloadService::OnDownloadEnd, base::Unretained(this)), base::Bind(&ReadingListDownloadService::OnDeleteEnd,
diff --git a/ios/chrome/browser/reading_list/reading_list_download_service.h b/ios/chrome/browser/reading_list/reading_list_download_service.h index 3f443f9..19b68f0 100644 --- a/ios/chrome/browser/reading_list/reading_list_download_service.h +++ b/ios/chrome/browser/reading_list/reading_list_download_service.h
@@ -40,6 +40,7 @@ dom_distiller::DomDistillerService* distiller_service, PrefService* prefs, base::FilePath chrome_profile_path, + net::URLRequestContextGetter* url_request_context_getter, std::unique_ptr<reading_list::ReadingListDistillerPageFactory> distiller_page_factory); ~ReadingListDownloadService() override;
diff --git a/ios/chrome/browser/reading_list/reading_list_download_service_factory.cc b/ios/chrome/browser/reading_list/reading_list_download_service_factory.cc index 874ec8ff3..33b9619 100644 --- a/ios/chrome/browser/reading_list/reading_list_download_service_factory.cc +++ b/ios/chrome/browser/reading_list/reading_list_download_service_factory.cc
@@ -62,6 +62,7 @@ chrome_browser_state), chrome_browser_state->GetPrefs(), chrome_browser_state->GetStatePath(), + chrome_browser_state->GetRequestContext(), std::move(distiller_page_factory))); return std::move(reading_list_download_service); }
diff --git a/ios/chrome/browser/reading_list/url_downloader.cc b/ios/chrome/browser/reading_list/url_downloader.cc index c50dfe7a..505dfbf 100644 --- a/ios/chrome/browser/reading_list/url_downloader.cc +++ b/ios/chrome/browser/reading_list/url_downloader.cc
@@ -18,6 +18,12 @@ #include "ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h" #include "ios/web/public/web_thread.h" #include "net/base/escape.h" +#include "net/base/load_flags.h" +#include "net/http/http_response_headers.h" +#include "net/url_request/url_fetcher.h" +#include "net/url_request/url_fetcher_delegate.h" +#include "net/url_request/url_request_context_getter.h" +#include "net/url_request/url_request_status.h" #include "url/gurl.h" namespace { @@ -41,6 +47,7 @@ reading_list::ReadingListDistillerPageFactory* distiller_page_factory, PrefService* prefs, base::FilePath chrome_profile_path, + net::URLRequestContextGetter* url_request_context_getter, const DownloadCompletion& download_completion, const SuccessCompletion& delete_completion) : distiller_service_(distiller_service), @@ -50,20 +57,19 @@ delete_completion_(delete_completion), working_(false), base_directory_(chrome_profile_path), + mime_type_(), + url_request_context_getter_(url_request_context_getter), task_tracker_() {} URLDownloader::~URLDownloader() { task_tracker_.TryCancelAll(); } -void URLDownloader::OfflineURLExists(const GURL& url, - base::Callback<void(bool)> callback) { +void URLDownloader::OfflinePathExists(const base::FilePath& path, + base::Callback<void(bool)> callback) { task_tracker_.PostTaskAndReplyWithResult( web::WebThread::GetTaskRunnerForThread(web::WebThread::FILE).get(), - FROM_HERE, - base::Bind(&base::PathExists, - reading_list::OfflinePageAbsolutePath(base_directory_, url)), - callback); + FROM_HERE, base::Bind(&base::PathExists, path), callback); } void URLDownloader::RemoveOfflineURL(const GURL& url) { @@ -87,33 +93,35 @@ tasks_.end()); } -void URLDownloader::DownloadCompletionHandler(const GURL& url, - const std::string& title, - SuccessState success) { +void URLDownloader::DownloadCompletionHandler( + const GURL& url, + const std::string& title, + const base::FilePath& offline_path, + SuccessState success) { DCHECK(working_); auto post_delete = base::Bind( [](URLDownloader* _this, const GURL& url, const std::string& title, - SuccessState success) { + const base::FilePath& offline_path, SuccessState success) { _this->download_completion_.Run(url, _this->distilled_url_, success, - reading_list::OfflinePagePath(url), - title); + offline_path, title); _this->distiller_.reset(); _this->working_ = false; _this->HandleNextTask(); }, - base::Unretained(this), url, title, success); + base::Unretained(this), url, title, offline_path, success); // If downloading failed, clean up any partial download. if (success == ERROR_RETRY || success == ERROR_PERMANENT) { + base::FilePath directory_path = + reading_list::OfflineURLDirectoryAbsolutePath(base_directory_, url); task_tracker_.PostTaskAndReply( web::WebThread::GetTaskRunnerForThread(web::WebThread::FILE).get(), FROM_HERE, base::Bind( [](const base::FilePath& offline_directory_path) { base::DeleteFile(offline_directory_path, true); }, - reading_list::OfflineURLDirectoryAbsolutePath( - base_directory_, url)), + directory_path), post_delete); } else { post_delete.Run(); @@ -136,26 +144,26 @@ Task task = tasks_.front(); tasks_.pop_front(); GURL url = task.second; + base::FilePath directory_path = + reading_list::OfflineURLDirectoryAbsolutePath(base_directory_, url); if (task.first == DELETE) { task_tracker_.PostTaskAndReplyWithResult( web::WebThread::GetTaskRunnerForThread(web::WebThread::FILE).get(), - FROM_HERE, base::Bind(&base::DeleteFile, - reading_list::OfflineURLDirectoryAbsolutePath( - base_directory_, url), - true), + FROM_HERE, base::Bind(&base::DeleteFile, directory_path, true), base::Bind(&URLDownloader::DeleteCompletionHandler, base::Unretained(this), url)); } else if (task.first == DOWNLOAD) { DCHECK(!distiller_); - OfflineURLExists(url, base::Bind(&URLDownloader::DownloadURL, - base::Unretained(this), url)); + OfflinePathExists(directory_path, base::Bind(&URLDownloader::DownloadURL, + base::Unretained(this), url)); } } -void URLDownloader::DownloadURL(GURL url, bool offline_url_exists) { +void URLDownloader::DownloadURL(const GURL& url, bool offline_url_exists) { if (offline_url_exists) { - DownloadCompletionHandler(url, std::string(), DOWNLOAD_EXISTS); + DownloadCompletionHandler(url, std::string(), base::FilePath(), + DOWNLOAD_EXISTS); return; } @@ -163,9 +171,7 @@ distilled_url_ = url; std::unique_ptr<reading_list::ReadingListDistillerPage> reading_list_distiller_page = - distiller_page_factory_->CreateReadingListDistillerPage(); - reading_list_distiller_page->SetRedirectionCallback( - base::Bind(&URLDownloader::RedirectionCallback, base::Unretained(this))); + distiller_page_factory_->CreateReadingListDistillerPage(this); distiller_.reset(new dom_distiller::DistillerViewer( distiller_service_, pref_service_, url, @@ -173,12 +179,74 @@ std::move(reading_list_distiller_page))); } -void URLDownloader::RedirectionCallback(const GURL& page_url, - const GURL& redirected_url) { +void URLDownloader::DistilledPageRedirectedToURL(const GURL& page_url, + const GURL& redirected_url) { DCHECK(original_url_ == page_url); distilled_url_ = redirected_url; } +void URLDownloader::DistilledPageHasMimeType(const GURL& original_url, + const std::string& mime_type) { + DCHECK(original_url_ == original_url); + mime_type_ = mime_type; +} + +void URLDownloader::OnURLFetchComplete(const net::URLFetcher* source) { + DCHECK(source == fetcher_.get()); + // At the moment, only pdf files are downloaded using URLFetcher. + DCHECK(mime_type_ == "application/pdf"); + base::FilePath path = reading_list::OfflinePagePath( + original_url_, reading_list::OFFLINE_TYPE_PDF); + std::string mime_type; + if (fetcher_->GetResponseHeaders()) { + fetcher_->GetResponseHeaders()->GetMimeType(&mime_type); + } + if (!fetcher_->GetStatus().is_success() || mime_type != mime_type_) { + return DownloadCompletionHandler(original_url_, "", path, ERROR_RETRY); + } + base::FilePath temporary_path; + // Do not take ownership of the file until the file is moved. This ensures + // that the file is cleaned if there a problem before file is moved. + fetcher_->GetResponseAsFilePath(false, &temporary_path); + + task_tracker_.PostTaskAndReplyWithResult( + web::WebThread::GetTaskRunnerForThread(web::WebThread::FILE).get(), + FROM_HERE, base::Bind(&URLDownloader::SavePDFFile, base::Unretained(this), + temporary_path), + base::Bind(&URLDownloader::DownloadCompletionHandler, + base::Unretained(this), source->GetOriginalURL(), "", path)); +} + +void URLDownloader::FetchPDFFile() { + const GURL& pdf_url = + distilled_url_.is_valid() ? distilled_url_ : original_url_; + fetcher_ = net::URLFetcher::Create(0, pdf_url, net::URLFetcher::GET, this); + fetcher_->SetRequestContext(url_request_context_getter_.get()); + fetcher_->SetLoadFlags(net::LOAD_SKIP_CACHE_VALIDATION); + fetcher_->SaveResponseToTemporaryFile( + web::WebThread::GetTaskRunnerForThread(web::WebThread::FILE)); + fetcher_->Start(); +} + +URLDownloader::SuccessState URLDownloader::SavePDFFile( + const base::FilePath& temporary_path) { + if (CreateOfflineURLDirectory(original_url_)) { + base::FilePath path = reading_list::OfflinePagePath( + original_url_, reading_list::OFFLINE_TYPE_PDF); + base::FilePath absolute_path = + reading_list::OfflineURLAbsolutePathFromRelativePath(base_directory_, + path); + + if (base::Move(temporary_path, absolute_path)) { + return DOWNLOAD_SUCCESS; + } else { + return ERROR_PERMANENT; + } + } + + return ERROR_PERMANENT; +} + void URLDownloader::DistillerCallback( const GURL& page_url, const std::string& html, @@ -186,7 +254,16 @@ images, const std::string& title) { if (html.empty()) { - DownloadCompletionHandler(page_url, std::string(), ERROR_RETRY); + // The page may not be HTML. Check the mime-type to see if another handler + // can save offline content + if (mime_type_ == "application/pdf") { + // PDF handler just downloads the PDF dfile + FetchPDFFile(); + return; + } + // This content cannot be processed, return an error value to the client. + DownloadCompletionHandler(page_url, std::string(), base::FilePath(), + ERROR_RETRY); return; } @@ -198,7 +275,9 @@ base::Bind(&URLDownloader::SaveDistilledHTML, base::Unretained(this), page_url, images_block, block_html), base::Bind(&URLDownloader::DownloadCompletionHandler, - base::Unretained(this), page_url, title)); + base::Unretained(this), page_url, title, + reading_list::OfflinePagePath( + page_url, reading_list::OFFLINE_TYPE_HTML))); } URLDownloader::SuccessState URLDownloader::SaveDistilledHTML( @@ -215,10 +294,10 @@ } bool URLDownloader::CreateOfflineURLDirectory(const GURL& url) { - base::FilePath path = + base::FilePath directory_path = reading_list::OfflineURLDirectoryAbsolutePath(base_directory_, url); - if (!DirectoryExists(path)) { - return CreateDirectoryAndGetError(path, nil); + if (!DirectoryExists(directory_path)) { + return CreateDirectoryAndGetError(directory_path, nil); } return true; } @@ -229,9 +308,9 @@ std::string* image_name) { std::string image_hash = base::MD5String(image_url.spec()); *image_name = image_hash; - base::FilePath path = - reading_list::OfflineURLDirectoryAbsolutePath(base_directory_, url) - .Append(image_hash); + base::FilePath directory_path = + reading_list::OfflineURLDirectoryAbsolutePath(base_directory_, url); + base::FilePath path = directory_path.Append(image_hash); if (!base::PathExists(path)) { return base::WriteFile(path, data.c_str(), data.length()) > 0; } @@ -280,7 +359,8 @@ if (html.empty()) { return false; } - base::FilePath path = - reading_list::OfflinePageAbsolutePath(base_directory_, url); + base::FilePath path = reading_list::OfflineURLAbsolutePathFromRelativePath( + base_directory_, + reading_list::OfflinePagePath(url, reading_list::OFFLINE_TYPE_HTML)); return base::WriteFile(path, html.c_str(), html.length()) > 0; }
diff --git a/ios/chrome/browser/reading_list/url_downloader.h b/ios/chrome/browser/reading_list/url_downloader.h index a497248..f9c26a3 100644 --- a/ios/chrome/browser/reading_list/url_downloader.h +++ b/ios/chrome/browser/reading_list/url_downloader.h
@@ -10,6 +10,7 @@ #include "base/callback.h" #include "base/task/cancelable_task_tracker.h" #include "ios/chrome/browser/dom_distiller/distiller_viewer.h" +#include "ios/chrome/browser/reading_list/reading_list_distiller_page.h" class PrefService; class GURL; @@ -21,25 +22,41 @@ class DomDistillerService; } +namespace net { +class URLFetcher; +class URLRequestContextGetter; +} + namespace reading_list { class ReadingListDistillerPageFactory; } -// This class downloads and deletes offline versions of URLs using DOM distiller -// to fetch the page and simplify it. Only one item is downloaded or deleted at -// a time using a queue of tasks that are handled sequentially. Items (page + -// images) are saved to individual folders within an offline folder, using md5 -// hashing to create unique file names. When a deletion is requested, all -// previous downloads for that URL are cancelled as they would be deleted. -class URLDownloader { +// This class downloads and deletes offline versions of URLs. +// If the URL points to an HTML file, |URLDownloader| uses DOM distiller to +// fetch the page and simplify it. +// If the URL points to a PDF file, the PDF is simply downloaded and saved to +// the disk. +// Only one item is downloaded or deleted at a time using a queue of tasks that +// are handled sequentially. Items (page + images) are saved to individual +// folders within an offline folder, using md5 hashing to create unique file +// names. When a deletion is requested, all previous downloads for that URL are +// cancelled as they would be deleted. +class URLDownloader : public net::URLFetcherDelegate, + reading_list::ReadingListDistillerPageDelegate { friend class MockURLDownloader; public: // And enum indicating different download outcomes. enum SuccessState { + // The URL was correctly downloaded and an offline version is not available. DOWNLOAD_SUCCESS, + // The URL was already available offline. No action was done. DOWNLOAD_EXISTS, + // The URL could not be downloaded because of a temporary error. Client may + // want to try again later. ERROR_RETRY, + // The URL could not be dowmloaded and URLDownloader thinks a retry would + // end with the same result. There is no need to retry. ERROR_PERMANENT }; @@ -69,9 +86,10 @@ reading_list::ReadingListDistillerPageFactory* distiller_page_factory, PrefService* prefs, base::FilePath chrome_profile_path, + net::URLRequestContextGetter* url_request_context_getter, const DownloadCompletion& download_completion, const SuccessCompletion& delete_completion); - virtual ~URLDownloader(); + ~URLDownloader() override; // Asynchronously download an offline version of the URL. void DownloadOfflineURL(const GURL& url); @@ -82,18 +100,24 @@ // Asynchronously remove the offline version of the URL if it exists. void RemoveOfflineURL(const GURL& url); + // URLFetcherDelegate delegate method. + void OnURLFetchComplete(const net::URLFetcher* source) override; + private: enum TaskType { DELETE, DOWNLOAD }; using Task = std::pair<TaskType, GURL>; - // Calls callback with true if an offline file exists for this URL. - void OfflineURLExists(const GURL& url, base::Callback<void(bool)> callback); + // Calls callback with true if an offline path exists. |path| must be + // absolute. + void OfflinePathExists(const base::FilePath& url, + base::Callback<void(bool)> callback); // Handles the next task in the queue, if no task is currently being handled. void HandleNextTask(); // Callback for completed (or failed) download, handles calling // downloadCompletion and starting the next task. void DownloadCompletionHandler(const GURL& url, const std::string& title, + const base::FilePath& path, SuccessState success); // Callback for completed (or failed) deletion, handles calling // deleteCompletion and starting the next task. @@ -102,6 +126,18 @@ // Creates the offline directory for |url|. Returns true if successful or if // the directory already exists. bool CreateOfflineURLDirectory(const GURL& url); + + // Downloads |url|, depending on |offlineURLExists| state. + virtual void DownloadURL(const GURL& url, bool offlineURLExists); + + // ReadingListDistillerPageDelegate methods + void DistilledPageRedirectedToURL(const GURL& original_url, + const GURL& final_url) override; + void DistilledPageHasMimeType(const GURL& original_url, + const std::string& mime_type) override; + + // HTML processing methods. + // Saves the |data| for image at |imageURL| to disk, for main URL |url|; // puts path of saved file in |path| and returns whether save was successful. bool SaveImage(const GURL& url, @@ -119,8 +155,6 @@ images); // Saves |html| to disk in the correct location for |url|; returns success. bool SaveHTMLForURL(std::string html, const GURL& url); - // Downloads |url|, depending on |offlineURLExists| state. - virtual void DownloadURL(GURL url, bool offlineURLExists); // Saves distilled html to disk, including saving images and main file. SuccessState SaveDistilledHTML( const GURL& url, @@ -135,8 +169,13 @@ images, const std::string& title); - // A callback called if the URL passed to the distilled led to a redirection. - void RedirectionCallback(const GURL& page_url, const GURL& redirected_url); + // PDF processing methods + + // Starts fetching the PDF file. If |original_url_| triggered a redirection, + // directly save |distilled_url_|. + virtual void FetchPDFFile(); + // Saves the file downloaded by |fetcher_|. Creates the directory if needed. + SuccessState SavePDFFile(const base::FilePath& temporary_path); dom_distiller::DomDistillerService* distiller_service_; reading_list::ReadingListDistillerPageFactory* distiller_page_factory_; @@ -148,6 +187,11 @@ base::FilePath base_directory_; GURL original_url_; GURL distilled_url_; + std::string mime_type_; + // Fetcher used to redownload the document and save it in the sandbox. + std::unique_ptr<net::URLFetcher> fetcher_; + // URLRequestContextGetter needed for the URLFetcher. + scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_; std::unique_ptr<dom_distiller::DistillerViewerInterface> distiller_; base::CancelableTaskTracker task_tracker_;
diff --git a/ios/chrome/browser/reading_list/url_downloader_unittest.mm b/ios/chrome/browser/reading_list/url_downloader_unittest.mm index 611f6ad..4d082cf 100644 --- a/ios/chrome/browser/reading_list/url_downloader_unittest.mm +++ b/ios/chrome/browser/reading_list/url_downloader_unittest.mm
@@ -15,6 +15,7 @@ #include "ios/chrome/browser/chrome_paths.h" #include "ios/chrome/browser/dom_distiller/distiller_viewer.h" #include "ios/chrome/browser/reading_list/offline_url_utils.h" +#include "ios/chrome/browser/reading_list/reading_list_distiller_page.h" #include "ios/web/public/test/test_web_thread_bundle.h" #include "testing/gtest/include/gtest/gtest.h" @@ -23,10 +24,20 @@ class DistillerViewerTest : public dom_distiller::DistillerViewerInterface { public: DistillerViewerTest(const GURL& url, - const DistillationFinishedCallback& callback) + const DistillationFinishedCallback& callback, + reading_list::ReadingListDistillerPageDelegate* delegate, + const std::string& html, + const GURL& redirect_url, + const std::string& mime_type) : dom_distiller::DistillerViewerInterface(nil, nil) { std::vector<ImageInfo> images; - callback.Run(url, "html", images, "title"); + if (redirect_url.is_valid()) { + delegate->DistilledPageRedirectedToURL(url, redirect_url); + } + if (!mime_type.empty()) { + delegate->DistilledPageHasMimeType(url, mime_type); + } + callback.Run(url, html, images, "title"); } void OnArticleReady( @@ -35,6 +46,11 @@ void SendJavaScript(const std::string& buffer) override {} }; +void RemoveOfflineFilesDirectory(base::FilePath base_directory) { + base::DeleteFile(reading_list::OfflineRootDirectoryPath(base_directory), + true); +} + } // namespace class MockURLDownloader : public URLDownloader { @@ -44,15 +60,12 @@ nullptr, nullptr, path, + nullptr, base::Bind(&MockURLDownloader::OnEndDownload, base::Unretained(this)), base::Bind(&MockURLDownloader::OnEndRemove, - base::Unretained(this))) {} - - void RemoveOfflineFilesDirectory() { - base::DeleteFile(reading_list::OfflineRootDirectoryPath(base_directory_), - true); - } + base::Unretained(this))), + html_("html") {} void ClearCompletionTrackers() { downloaded_files_.clear(); @@ -61,7 +74,9 @@ bool CheckExistenceOfOfflineURLPagePath(const GURL& url) { return base::PathExists( - reading_list::OfflinePageAbsolutePath(base_directory_, url)); + reading_list::OfflineURLAbsolutePathFromRelativePath( + base_directory_, reading_list::OfflinePagePath( + url, reading_list::OFFLINE_TYPE_HTML))); } void FakeWorking() { working_ = true; } @@ -73,24 +88,34 @@ std::vector<GURL> downloaded_files_; std::vector<GURL> removed_files_; + GURL redirect_url_; + std::string mime_type_; + std::string html_; + bool fetching_pdf_; private: - void DownloadURL(GURL url, bool offlineURLExists) override { - if (offlineURLExists) { - DownloadCompletionHandler(url, std::string(), DOWNLOAD_EXISTS); + void DownloadURL(const GURL& url, bool offline_url_exists) override { + if (offline_url_exists) { + DownloadCompletionHandler(url, std::string(), base::FilePath(), + DOWNLOAD_EXISTS); return; } + original_url_ = url; distiller_.reset(new DistillerViewerTest( url, - base::Bind(&URLDownloader::DistillerCallback, base::Unretained(this)))); + base::Bind(&URLDownloader::DistillerCallback, base::Unretained(this)), + this, html_, redirect_url_, mime_type_)); } + void FetchPDFFile() override { fetching_pdf_ = true; } + void OnEndDownload(const GURL& url, const GURL& distilled_url, SuccessState success, const base::FilePath& distilled_path, const std::string& title) { downloaded_files_.push_back(url); + DCHECK_EQ(distilled_url, redirect_url_); } void OnEndRemove(const GURL& url, bool success) { @@ -107,12 +132,15 @@ URLDownloaderTest() { base::FilePath data_dir; base::PathService::Get(ios::DIR_USER_DATA, &data_dir); + RemoveOfflineFilesDirectory(data_dir); downloader_.reset(new MockURLDownloader(data_dir)); } ~URLDownloaderTest() override {} void TearDown() override { - downloader_->RemoveOfflineFilesDirectory(); + base::FilePath data_dir; + base::PathService::Get(ios::DIR_USER_DATA, &data_dir); + RemoveOfflineFilesDirectory(data_dir); downloader_->ClearCompletionTrackers(); } @@ -139,6 +167,45 @@ ASSERT_TRUE(downloader_->CheckExistenceOfOfflineURLPagePath(url)); } +TEST_F(URLDownloaderTest, SingleDownloadRedirect) { + GURL url = GURL("http://test.com"); + ASSERT_FALSE(downloader_->CheckExistenceOfOfflineURLPagePath(url)); + ASSERT_EQ(0ul, downloader_->downloaded_files_.size()); + ASSERT_EQ(0ul, downloader_->removed_files_.size()); + + // The DCHECK in OnEndDownload will verify that the redirection was handled + // correctly. + downloader_->redirect_url_ = GURL("http://test.com/redirected"); + + downloader_->DownloadOfflineURL(url); + + WaitUntilCondition(^bool { + return std::find(downloader_->downloaded_files_.begin(), + downloader_->downloaded_files_.end(), + url) != downloader_->downloaded_files_.end(); + }); + + EXPECT_TRUE(downloader_->CheckExistenceOfOfflineURLPagePath(url)); +} + +TEST_F(URLDownloaderTest, SingleDownloadPDF) { + GURL url = GURL("http://test.com"); + ASSERT_FALSE(downloader_->CheckExistenceOfOfflineURLPagePath(url)); + ASSERT_EQ(0ul, downloader_->downloaded_files_.size()); + ASSERT_EQ(0ul, downloader_->removed_files_.size()); + + downloader_->mime_type_ = "application/pdf"; + downloader_->html_ = ""; + + downloader_->DownloadOfflineURL(url); + + WaitUntilCondition(^bool { + return downloader_->fetching_pdf_; + }); + + EXPECT_FALSE(downloader_->CheckExistenceOfOfflineURLPagePath(url)); +} + TEST_F(URLDownloaderTest, DownloadAndRemove) { GURL url = GURL("http://test.com"); GURL url2 = GURL("http://test2.com");
diff --git a/ios/chrome/browser/resources/Settings.bundle/Experimental.plist b/ios/chrome/browser/resources/Settings.bundle/Experimental.plist index 9d6dddb..56b1c485 100644 --- a/ios/chrome/browser/resources/Settings.bundle/Experimental.plist +++ b/ios/chrome/browser/resources/Settings.bundle/Experimental.plist
@@ -336,6 +336,28 @@ <key>DefaultValue</key> <false/> </dict> + <dict> + <key>Type</key> + <string>PSMultiValueSpecifier</string> + <key>Title</key> + <string>Enable Suggestions UI</string> + <key>Key</key> + <string>EnableSuggestions</string> + <key>DefaultValue</key> + <string></string> + <key>Values</key> + <array> + <string></string> + <string>Enabled</string> + <string>Disabled</string> + </array> + <key>Titles</key> + <array> + <string>Default</string> + <string>Enabled</string> + <string>Disabled</string> + </array> + </dict> <dict> <key>Type</key> <string>PSMultiValueSpecifier</string>
diff --git a/ios/chrome/browser/ui/BUILD.gn b/ios/chrome/browser/ui/BUILD.gn index 8cdae1d..5927606e 100644 --- a/ios/chrome/browser/ui/BUILD.gn +++ b/ios/chrome/browser/ui/BUILD.gn
@@ -252,6 +252,7 @@ "//ios/chrome/browser", "//ios/chrome/browser/bookmarks", "//ios/chrome/browser/browser_state", + "//ios/chrome/browser/content_suggestions", "//ios/chrome/browser/favicon", "//ios/chrome/browser/find_in_page", "//ios/chrome/browser/first_run",
diff --git a/ios/chrome/browser/ui/browser_view_controller.mm b/ios/chrome/browser/ui/browser_view_controller.mm index 19d7cc9c..3db83f72 100644 --- a/ios/chrome/browser/ui/browser_view_controller.mm +++ b/ios/chrome/browser/ui/browser_view_controller.mm
@@ -50,6 +50,7 @@ #include "ios/chrome/browser/browser_state/chrome_browser_state.h" #include "ios/chrome/browser/chrome_url_constants.h" #include "ios/chrome/browser/chrome_url_util.h" +#import "ios/chrome/browser/content_suggestions/content_suggestions_coordinator.h" #include "ios/chrome/browser/experimental_flags.h" #import "ios/chrome/browser/favicon/favicon_loader.h" #include "ios/chrome/browser/favicon/ios_chrome_favicon_loader_factory.h" @@ -385,6 +386,10 @@ // Used to display the QR Scanner UI. Nil if not visible. base::scoped_nsobject<QRScannerViewController> _qrScannerViewController; + // Used to display the Suggestions. + base::scoped_nsobject<ContentSuggestionsCoordinator> + _contentSuggestionsCoordinator; + // Used to display the Find In Page UI. Nil if not visible. base::scoped_nsobject<FindBarControllerIOS> _findBarController; @@ -4127,6 +4132,11 @@ [self showQRScanner]; } break; + case IDC_SHOW_SUGGESTIONS: + if (experimental_flags::IsSuggestionsUIEnabled()) { + [self showSuggestionsUI]; + } + break; default: // Unknown commands get sent up the responder chain. [super chromeExecuteCommand:sender]; @@ -4232,6 +4242,8 @@ [_contextMenuCoordinator stop]; [self dismissRateThisAppDialog]; + [_contentSuggestionsCoordinator stop]; + if (self.presentedViewController) { // Dismisses any other modal controllers that may be present, e.g. Recent // Tabs. @@ -4392,6 +4404,14 @@ completion:nil]; } +- (void)showSuggestionsUI { + if (!_contentSuggestionsCoordinator) { + _contentSuggestionsCoordinator.reset([[ContentSuggestionsCoordinator alloc] + initWithBaseViewController:self]); + } + [_contentSuggestionsCoordinator start]; +} + - (void)showNTPPanel:(NewTabPage::PanelIdentifier)panel { DCHECK(self.visible || self.dismissingModal); GURL url(kChromeUINewTabURL);
diff --git a/ios/chrome/browser/ui/commands/ios_command_ids.h b/ios/chrome/browser/ui/commands/ios_command_ids.h index d78b516..4d16f374 100644 --- a/ios/chrome/browser/ui/commands/ios_command_ids.h +++ b/ios/chrome/browser/ui/commands/ios_command_ids.h
@@ -76,6 +76,7 @@ #define IDC_SHOW_SYNC_PASSPHRASE_SETTINGS 40952 #define IDC_SHOW_QR_SCANNER 40953 #define IDC_SHOW_PHYSICAL_WEB_SETTINGS 40954 +#define IDC_SHOW_SUGGESTIONS 40955 // clang-format on #endif // IOS_CHROME_BROWSER_UI_COMMANDS_IOS_COMMAND_IDS_H_
diff --git a/ios/chrome/browser/ui/content_suggestions/BUILD.gn b/ios/chrome/browser/ui/content_suggestions/BUILD.gn new file mode 100644 index 0000000..c35d965 --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/BUILD.gn
@@ -0,0 +1,58 @@ +# Copyright 2016 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +source_set("content_suggestions") { + configs += [ "//build/config/compiler:enable_arc" ] + sources = [ + "content_suggestions_article_item.h", + "content_suggestions_article_item.mm", + "content_suggestions_collection_updater.h", + "content_suggestions_collection_updater.mm", + "content_suggestions_commands.h", + "content_suggestions_expandable_item.h", + "content_suggestions_expandable_item.mm", + "content_suggestions_favicon_internal_cell.h", + "content_suggestions_favicon_internal_cell.mm", + "content_suggestions_favicon_item.h", + "content_suggestions_favicon_item.mm", + "content_suggestions_item.h", + "content_suggestions_item.mm", + "content_suggestions_item_actions.h", + "content_suggestions_stack_item.h", + "content_suggestions_stack_item.mm", + "content_suggestions_stack_item_actions.h", + "content_suggestions_view_controller.h", + "content_suggestions_view_controller.mm", + "expandable_item.h", + ] + deps = [ + "//base", + "//ios/chrome/browser/ui", + "//ios/chrome/browser/ui/collection_view", + "//ios/third_party/material_roboto_font_loader_ios", + "//ui/base", + ] + public_deps = [ + "//ios/third_party/material_components_ios", + ] +} + +source_set("unit_tests") { + configs += [ "//build/config/compiler:enable_arc" ] + testonly = true + sources = [ + "content_suggestions_article_item_unittest.mm", + "content_suggestions_expandable_item_unittest.mm", + "content_suggestions_favicon_item_unittest.mm", + "content_suggestions_item_unittest.mm", + "content_suggestions_stack_item_unittest.mm", + ] + deps = [ + ":content_suggestions", + "//base", + "//ios/chrome/browser/ui/collection_view", + "//testing/gtest", + "//third_party/ocmock", + ] +}
diff --git a/ios/chrome/browser/ui/suggestions/OWNERS b/ios/chrome/browser/ui/content_suggestions/OWNERS similarity index 100% rename from ios/chrome/browser/ui/suggestions/OWNERS rename to ios/chrome/browser/ui/content_suggestions/OWNERS
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_article_item.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.h similarity index 72% rename from ios/chrome/browser/ui/suggestions/suggestions_article_item.h rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.h index bcf04038..7415da21 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_article_item.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.h
@@ -2,14 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_ARTICLE_ITEM_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_ARTICLE_ITEM_H_ +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_ARTICLE_ITEM_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_ARTICLE_ITEM_H_ #import "ios/chrome/browser/ui/collection_view/cells/collection_view_item.h" #import "ios/third_party/material_components_ios/src/components/CollectionCells/src/MaterialCollectionCells.h" // Item for an article in the suggestions. -@interface SuggestionsArticleItem : CollectionViewItem +@interface ContentSuggestionsArticleItem : CollectionViewItem // Initialize an article with a |title|, a |subtitle| and an |image|. |type| is // the type of the item. @@ -23,7 +23,7 @@ @end // Corresponding cell for an article in the suggestions. -@interface SuggestionsArticleCell : MDCCollectionViewCell +@interface ContentSuggestionsArticleCell : MDCCollectionViewCell @property(nonatomic, readonly, strong) UILabel* titleLabel; @property(nonatomic, readonly, strong) UILabel* subtitleLabel; @@ -31,4 +31,4 @@ @end -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_ARTICLE_ITEM_H_ +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_ARTICLE_ITEM_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_article_item.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.mm similarity index 89% rename from ios/chrome/browser/ui/suggestions/suggestions_article_item.mm rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.mm index a26ee1b..a87624da 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_article_item.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.mm
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#import "ios/chrome/browser/ui/suggestions/suggestions_article_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.h" #import "ios/chrome/browser/ui/uikit_ui_util.h" @@ -14,7 +14,7 @@ const CGFloat kImageSize = 100; } -@interface SuggestionsArticleItem () +@interface ContentSuggestionsArticleItem () @property(nonatomic, copy) NSString* title; @property(nonatomic, copy) NSString* subtitle; @@ -22,7 +22,7 @@ @end -@implementation SuggestionsArticleItem +@implementation ContentSuggestionsArticleItem @synthesize title = _title; @synthesize subtitle = _subtitle; @@ -34,7 +34,7 @@ image:(UIImage*)image { self = [super initWithType:type]; if (self) { - self.cellClass = [SuggestionsArticleCell class]; + self.cellClass = [ContentSuggestionsArticleCell class]; _title = [title copy]; _subtitle = [subtitle copy]; _image = image; @@ -42,7 +42,7 @@ return self; } -- (void)configureCell:(SuggestionsArticleCell*)cell { +- (void)configureCell:(ContentSuggestionsArticleCell*)cell { [super configureCell:cell]; cell.titleLabel.text = _title; cell.subtitleLabel.text = _subtitle; @@ -51,7 +51,7 @@ @end -@implementation SuggestionsArticleCell +@implementation ContentSuggestionsArticleCell @synthesize titleLabel = _titleLabel; @synthesize subtitleLabel = _subtitleLabel;
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item_unittest.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item_unittest.mm new file mode 100644 index 0000000..dde70de --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item_unittest.mm
@@ -0,0 +1,34 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.h" + +#include "testing/gtest/include/gtest/gtest.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace { + +// Tests that configureCell: set all the fields of the cell. +TEST(ContentSuggestionsArticleItemTest, CellIsConfigured) { + NSString* title = @"testTitle"; + NSString* subtitle = @"testSubtitle"; + UIImage* image = [[UIImage alloc] init]; + ContentSuggestionsArticleItem* item = + [[ContentSuggestionsArticleItem alloc] initWithType:0 + title:title + subtitle:subtitle + image:image]; + ContentSuggestionsArticleCell* cell = [[[item cellClass] alloc] init]; + EXPECT_EQ([ContentSuggestionsArticleCell class], [cell class]); + + [item configureCell:cell]; + EXPECT_EQ(title, cell.titleLabel.text); + EXPECT_EQ(subtitle, cell.subtitleLabel.text); + EXPECT_EQ(image, cell.imageView.image); +} + +} // namespace
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_collection_updater.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h similarity index 68% rename from ios/chrome/browser/ui/suggestions/suggestions_collection_updater.h rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h index 31a0c8977..b453f17 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_collection_updater.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h
@@ -2,16 +2,16 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_COLLECTION_UPDATER_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_COLLECTION_UPDATER_H_ +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COLLECTION_UPDATER_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COLLECTION_UPDATER_H_ #import <UIKit/UIKit.h> #import "ios/chrome/browser/ui/collection_view/collection_view_model.h" -@class SuggestionsViewController; +@class ContentSuggestionsViewController; -// Enum defining the ItemType of this SuggestionsCollectionUpdater. +// Enum defining the ItemType of this ContentSuggestionsCollectionUpdater. typedef NS_ENUM(NSInteger, ItemType) { ItemTypeText = kItemTypeEnumZero, ItemTypeArticle, @@ -22,12 +22,12 @@ // Updater for a CollectionViewController populating it with some items and // handling the items addition. -@interface SuggestionsCollectionUpdater : NSObject +@interface ContentSuggestionsCollectionUpdater : NSObject // |collectionViewController| this Updater will update. Needs to be set before // adding items. @property(nonatomic, assign) - SuggestionsViewController* collectionViewController; + ContentSuggestionsViewController* collectionViewController; // Adds a text item with a |title| and a |subtitle| in the section numbered // |section|. If |section| is greater than the current number of section, it @@ -41,4 +41,4 @@ @end -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_COLLECTION_UPDATER_H_ +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COLLECTION_UPDATER_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_collection_updater.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm similarity index 76% rename from ios/chrome/browser/ui/suggestions/suggestions_collection_updater.mm rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm index 1400e88..6ba3bf0d 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_collection_updater.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm
@@ -2,31 +2,31 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#import "ios/chrome/browser/ui/suggestions/suggestions_collection_updater.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h" #include "base/logging.h" #include "base/mac/foundation_util.h" #import "ios/chrome/browser/ui/collection_view/collection_view_controller.h" #import "ios/chrome/browser/ui/collection_view/collection_view_model.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_article_item.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_expandable_item.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_favicon_item.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_item.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_stack_item.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_view_controller.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." #endif -@implementation SuggestionsCollectionUpdater +@implementation ContentSuggestionsCollectionUpdater @synthesize collectionViewController = _collectionViewController; #pragma mark - Properties - (void)setCollectionViewController: - (SuggestionsViewController*)collectionViewController { + (ContentSuggestionsViewController*)collectionViewController { _collectionViewController = collectionViewController; [collectionViewController loadModel]; CollectionViewModel* model = collectionViewController.collectionViewModel; @@ -34,15 +34,16 @@ // Stack Item. [model addSectionWithIdentifier:sectionIdentifier]; - [model addItem:[[SuggestionsStackItem alloc] initWithType:ItemTypeStack - title:@"The title" - subtitle:@"The subtitle"] + [model addItem:[[ContentSuggestionsStackItem alloc] + initWithType:ItemTypeStack + title:@"The title" + subtitle:@"The subtitle"] toSectionWithIdentifier:sectionIdentifier++]; // Favicon Item. [model addSectionWithIdentifier:sectionIdentifier]; - SuggestionsFaviconItem* faviconItem = - [[SuggestionsFaviconItem alloc] initWithType:ItemTypeFavicon]; + ContentSuggestionsFaviconItem* faviconItem = + [[ContentSuggestionsFaviconItem alloc] initWithType:ItemTypeFavicon]; for (NSInteger i = 0; i < 6; i++) { [faviconItem addFavicon:[UIImage imageNamed:@"bookmark_gray_star"] withTitle:@"Super website! Incredible!"]; @@ -54,13 +55,13 @@ [model addSectionWithIdentifier:sectionIdentifier]; // Standard Item. - [model addItem:[[SuggestionsItem alloc] initWithType:ItemTypeText - title:@"The title" - subtitle:@"The subtitle"] + [model addItem:[[ContentSuggestionsItem alloc] initWithType:ItemTypeText + title:@"The title" + subtitle:@"The subtitle"] toSectionWithIdentifier:sectionIdentifier]; // Article Item. - [model addItem:[[SuggestionsArticleItem alloc] + [model addItem:[[ContentSuggestionsArticleItem alloc] initWithType:ItemTypeArticle title:@"Title of an Article" subtitle:@"This is the subtitle of an article, can " @@ -93,9 +94,10 @@ subtitle:(NSString*)subtitle toSection:(NSInteger)inputSection { DCHECK(_collectionViewController); - SuggestionsItem* item = [[SuggestionsItem alloc] initWithType:ItemTypeText - title:title - subtitle:subtitle]; + ContentSuggestionsItem* item = + [[ContentSuggestionsItem alloc] initWithType:ItemTypeText + title:title + subtitle:subtitle]; NSInteger sectionIdentifier = kSectionIdentifierEnumZero + inputSection; NSInteger sectionIndex = inputSection; CollectionViewModel* model = _collectionViewController.collectionViewModel;
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_commands.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_commands.h new file mode 100644 index 0000000..7a229627 --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_commands.h
@@ -0,0 +1,22 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COMMANDS_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COMMANDS_H_ + +// Commands protocol for the ContentSuggestionsViewController. +@protocol ContentSuggestionsCommands + +// Adds a new empty SuggestionItem. +- (void)addEmptyItem; +// Opens the Reading List. +- (void)openReadingList; +// Opens the first page of the Reading List. +- (void)openFirstPageOfReadingList; +// Opens the favicon associated with the cell with the |index|. +- (void)openFaviconAtIndex:(NSInteger)index; + +@end + +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COMMANDS_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_expandable_item.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.h similarity index 62% rename from ios/chrome/browser/ui/suggestions/suggestions_expandable_item.h rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.h index 174ab5c8..4e505282 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_expandable_item.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.h
@@ -2,18 +2,18 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_EXPANDABLE_ITEM_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_EXPANDABLE_ITEM_H_ +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_EXPANDABLE_ITEM_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_EXPANDABLE_ITEM_H_ #import "ios/chrome/browser/ui/collection_view/cells/collection_view_item.h" -#import "ios/chrome/browser/ui/suggestions/expandable_item.h" +#import "ios/chrome/browser/ui/content_suggestions/expandable_item.h" #import "ios/third_party/material_components_ios/src/components/CollectionCells/src/MaterialCollectionCells.h" -@class SuggestionsExpandableCell; +@class ContentSuggestionsExpandableCell; -// Delegate for the SuggestionsExpandableCell. The delegate will take care of -// the expansion/collapsing of the cell. -@protocol SuggestionsExpandableCellDelegate +// Delegate for the ContentSuggestionsExpandableCell. The delegate will take +// care of the expansion/collapsing of the cell. +@protocol ContentSuggestionsExpandableCellDelegate - (void)expandCell:(UICollectionViewCell*)cell; - (void)collapseCell:(UICollectionViewCell*)cell; @@ -22,8 +22,7 @@ // Item for an expandable article in the suggestions. An expandable article can // be expanded, displaying more informations/interactions. -@interface SuggestionsExpandableItem - : CollectionViewItem<SuggestionsExpandableArticle> +@interface SuggestionsExpandableItem : CollectionViewItem<ExpandableItem> // Init the article with a |title|, a |subtitle| an |image| and some |detail| // displayed only when the article is expanded. |type| is the type of the item. @@ -36,14 +35,16 @@ - (instancetype)initWithType:(NSInteger)type NS_UNAVAILABLE; // Delegate for the configured cells. -@property(nonatomic, weak) id<SuggestionsExpandableCellDelegate> delegate; +@property(nonatomic, weak) id<ContentSuggestionsExpandableCellDelegate> + delegate; @end // Corresponding cell of an expandable article. -@interface SuggestionsExpandableCell : MDCCollectionViewCell +@interface ContentSuggestionsExpandableCell : MDCCollectionViewCell -@property(nonatomic, weak) id<SuggestionsExpandableCellDelegate> delegate; +@property(nonatomic, weak) id<ContentSuggestionsExpandableCellDelegate> + delegate; @property(nonatomic, readonly, strong) UILabel* titleLabel; @property(nonatomic, readonly, strong) UILabel* subtitleLabel; @property(nonatomic, readonly, strong) UILabel* detailLabel; @@ -54,4 +55,4 @@ @end -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_EXPANDABLE_ITEM_H_ +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_EXPANDABLE_ITEM_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_expandable_item.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.mm similarity index 94% rename from ios/chrome/browser/ui/suggestions/suggestions_expandable_item.mm rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.mm index 761294e..5e20644 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_expandable_item.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.mm
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#import "ios/chrome/browser/ui/suggestions/suggestions_expandable_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.h" #import "ios/chrome/browser/ui/uikit_ui_util.h" #include "ui/base/l10n/l10n_util_mac.h" @@ -35,7 +35,7 @@ detailText:(NSString*)detail { self = [super initWithType:type]; if (self) { - self.cellClass = [SuggestionsExpandableCell class]; + self.cellClass = [ContentSuggestionsExpandableCell class]; _title = [title copy]; _subtitle = [subtitle copy]; _image = image; @@ -46,7 +46,7 @@ #pragma mark - CollectionViewItem -- (void)configureCell:(SuggestionsExpandableCell*)cell { +- (void)configureCell:(ContentSuggestionsExpandableCell*)cell { [super configureCell:cell]; cell.delegate = self.delegate; cell.titleLabel.text = _title; @@ -61,9 +61,9 @@ @end -#pragma mark - SuggestionsExpandableCell +#pragma mark - ContentSuggestionsExpandableCell -@interface SuggestionsExpandableCell () { +@interface ContentSuggestionsExpandableCell () { UIView* _articleContainer; UIButton* _interactionButton; UIButton* _expandButton; @@ -79,7 +79,7 @@ @end -@implementation SuggestionsExpandableCell +@implementation ContentSuggestionsExpandableCell @synthesize titleLabel = _titleLabel; @synthesize subtitleLabel = _subtitleLabel;
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_expandable_item_unittest.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item_unittest.mm similarity index 77% rename from ios/chrome/browser/ui/suggestions/suggestions_expandable_item_unittest.mm rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item_unittest.mm index 0fe5c97..bd858843 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_expandable_item_unittest.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item_unittest.mm
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#import "ios/chrome/browser/ui/suggestions/suggestions_expandable_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.h" #include "testing/gtest/include/gtest/gtest.h" #import "third_party/ocmock/OCMock/OCMock.h" @@ -12,15 +12,16 @@ #error "This file requires ARC support." #endif -// Test subclass of the SuggestionsExpandableCell. -@interface TestSuggestionsExpandableCell : SuggestionsExpandableCell +// Test subclass of the ContentSuggestionsExpandableCell. +@interface TestContentSuggestionsExpandableCell + : ContentSuggestionsExpandableCell @property(nonatomic) BOOL expandCalled; @property(nonatomic) BOOL collapseCalled; @end -@implementation TestSuggestionsExpandableCell +@implementation TestContentSuggestionsExpandableCell @synthesize expandCalled; @synthesize collapseCalled; @@ -46,7 +47,7 @@ UIImage* image = [[UIImage alloc] init]; NSString* details = @"testDetails"; id mockDelegate = [OCMockObject - mockForProtocol:@protocol(SuggestionsExpandableCellDelegate)]; + mockForProtocol:@protocol(ContentSuggestionsExpandableCellDelegate)]; SuggestionsExpandableItem* item = [[SuggestionsExpandableItem alloc] initWithType:0 @@ -55,8 +56,8 @@ image:image detailText:details]; item.delegate = mockDelegate; - SuggestionsExpandableCell* cell = [[[item cellClass] alloc] init]; - EXPECT_TRUE([cell isMemberOfClass:[SuggestionsExpandableCell class]]); + ContentSuggestionsExpandableCell* cell = [[[item cellClass] alloc] init]; + EXPECT_EQ([ContentSuggestionsExpandableCell class], [cell class]); [item configureCell:cell]; EXPECT_EQ(title, cell.titleLabel.text); @@ -75,9 +76,9 @@ subtitle:@"subtitle" image:nil detailText:@"detail"]; - TestSuggestionsExpandableCell* cell = - [[TestSuggestionsExpandableCell alloc] init]; - item.cellClass = [TestSuggestionsExpandableCell class]; + TestContentSuggestionsExpandableCell* cell = + [[TestContentSuggestionsExpandableCell alloc] init]; + item.cellClass = [TestContentSuggestionsExpandableCell class]; item.expanded = YES; [item configureCell:cell]; @@ -94,9 +95,9 @@ subtitle:@"subtitle" image:nil detailText:@"detail"]; - TestSuggestionsExpandableCell* cell = - [[TestSuggestionsExpandableCell alloc] init]; - item.cellClass = [TestSuggestionsExpandableCell class]; + TestContentSuggestionsExpandableCell* cell = + [[TestContentSuggestionsExpandableCell alloc] init]; + item.cellClass = [TestContentSuggestionsExpandableCell class]; item.expanded = NO; [item configureCell:cell];
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_internal_cell.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_internal_cell.h new file mode 100644 index 0000000..811511d --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_internal_cell.h
@@ -0,0 +1,20 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_FAVICON_INTERNAL_CELL_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_FAVICON_INTERNAL_CELL_H_ + +#import <UIKit/UIKit.h> + +// Cell to display a favicon in the internal collection view. +@interface ContentSuggestionsFaviconInternalCell : UICollectionViewCell + +@property(nonatomic, strong) UIImageView* faviconView; +@property(nonatomic, strong) UILabel* titleLabel; + ++ (NSString*)reuseIdentifier; + +@end + +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_FAVICON_INTERNAL_CELL_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_favicon_internal_cell.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_internal_cell.mm similarity index 92% rename from ios/chrome/browser/ui/suggestions/suggestions_favicon_internal_cell.mm rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_internal_cell.mm index 384d6821..e00c16b 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_favicon_internal_cell.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_internal_cell.mm
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#import "ios/chrome/browser/ui/suggestions/suggestions_favicon_internal_cell.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_internal_cell.h" #import "ios/chrome/browser/ui/uikit_ui_util.h" #import "ios/third_party/material_components_ios/src/components/Typography/src/MaterialTypography.h" @@ -16,7 +16,7 @@ const CGFloat kFontSize = 10; } -@implementation SuggestionsFaviconInternalCell +@implementation ContentSuggestionsFaviconInternalCell @synthesize faviconView = _faviconView; @synthesize titleLabel = _titleLabel;
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.h new file mode 100644 index 0000000..9e61d5ff --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.h
@@ -0,0 +1,42 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_FAVICON_ITEM_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_FAVICON_ITEM_H_ + +#import "ios/chrome/browser/ui/collection_view/cells/collection_view_item.h" +#import "ios/third_party/material_components_ios/src/components/CollectionCells/src/MaterialCollectionCells.h" + +// Delegate for the ContentSuggestionsFaviconCell, handling the interaction with +// the inner collection view. +@protocol ContentSuggestionsFaviconCellDelegate + +// Opens the favicon associated with the |indexPath| of the internal cell. +- (void)openFaviconAtIndexPath:(NSIndexPath*)innerIndexPath; + +@end + +// Item for a cell containing favicons in the suggestions. The favicons are +// presented in an inner collection view contained in the cell associated with +// this item. The collection view scrolls horizontally. +@interface ContentSuggestionsFaviconItem : CollectionViewItem + +@property(nonatomic, weak) id<ContentSuggestionsFaviconCellDelegate> delegate; + +// Adds a favicon with a |favicon| image and a |title| to this item. +- (void)addFavicon:(UIImage*)favicon withTitle:(NSString*)title; + +@end + +// The corresponding cell of a favicon item. +@interface ContentSuggestionsFaviconCell : MDCCollectionViewCell + +// The inner collection view, used to display the favicons. +@property(nonatomic, strong) UICollectionView* collectionView; + +@property(nonatomic, weak) id<ContentSuggestionsFaviconCellDelegate> delegate; + +@end + +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_FAVICON_ITEM_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_favicon_item.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.mm similarity index 79% rename from ios/chrome/browser/ui/suggestions/suggestions_favicon_item.mm rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.mm index 0a26ba41..64e22f7 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_favicon_item.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.mm
@@ -2,12 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#import "ios/chrome/browser/ui/suggestions/suggestions_favicon_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.h" #import <UIKit/UIKit.h> #include "base/logging.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_favicon_internal_cell.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_internal_cell.h" #import "ios/chrome/browser/ui/uikit_ui_util.h" #if !defined(__has_feature) || !__has_feature(objc_arc) @@ -28,21 +28,21 @@ #pragma mark - SugggestionsFaviconItem // The item is the data source of the inner collection view. -@interface SuggestionsFaviconItem ()<UICollectionViewDataSource> { +@interface ContentSuggestionsFaviconItem ()<UICollectionViewDataSource> { NSMutableArray<NSString*>* _faviconTitles; NSMutableArray<UIImage*>* _faviconImages; } @end -@implementation SuggestionsFaviconItem +@implementation ContentSuggestionsFaviconItem @synthesize delegate = _delegate; - (instancetype)initWithType:(NSInteger)type { self = [super initWithType:type]; if (self) { - self.cellClass = [SuggestionsFaviconCell class]; + self.cellClass = [ContentSuggestionsFaviconCell class]; _faviconTitles = [NSMutableArray array]; _faviconImages = [NSMutableArray array]; } @@ -58,9 +58,9 @@ - (UICollectionViewCell*)collectionView:(UICollectionView*)collectionView cellForItemAtIndexPath:(NSIndexPath*)indexPath { - SuggestionsFaviconInternalCell* cell = [collectionView - dequeueReusableCellWithReuseIdentifier:[SuggestionsFaviconInternalCell - reuseIdentifier] + ContentSuggestionsFaviconInternalCell* cell = [collectionView + dequeueReusableCellWithReuseIdentifier: + [ContentSuggestionsFaviconInternalCell reuseIdentifier] forIndexPath:indexPath]; cell.faviconView.image = [_faviconImages objectAtIndex:indexPath.item]; cell.titleLabel.text = [_faviconTitles objectAtIndex:indexPath.item]; @@ -75,7 +75,7 @@ #pragma mark - CollectionViewItem -- (void)configureCell:(SuggestionsFaviconCell*)cell { +- (void)configureCell:(ContentSuggestionsFaviconCell*)cell { [super configureCell:cell]; cell.collectionView.dataSource = self; cell.delegate = self.delegate; @@ -83,14 +83,14 @@ @end -#pragma mark - SuggestionsFaviconCell +#pragma mark - ContentSuggestionsFaviconCell // The cell is the delegate of the inner collection view. -@interface SuggestionsFaviconCell ()<UICollectionViewDelegate> +@interface ContentSuggestionsFaviconCell ()<UICollectionViewDelegate> @end -@implementation SuggestionsFaviconCell +@implementation ContentSuggestionsFaviconCell @synthesize collectionView = _collectionView; @synthesize delegate = _delegate; @@ -107,8 +107,8 @@ _collectionView = [[UICollectionView alloc] initWithFrame:CGRectZero collectionViewLayout:layout]; - [_collectionView registerClass:[SuggestionsFaviconInternalCell class] - forCellWithReuseIdentifier:[SuggestionsFaviconInternalCell + [_collectionView registerClass:[ContentSuggestionsFaviconInternalCell class] + forCellWithReuseIdentifier:[ContentSuggestionsFaviconInternalCell reuseIdentifier]]; _collectionView.backgroundColor = [UIColor clearColor]; _collectionView.translatesAutoresizingMaskIntoConstraints = NO;
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item_unittest.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item_unittest.mm new file mode 100644 index 0000000..d59ecec --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item_unittest.mm
@@ -0,0 +1,75 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.h" + +#include "base/mac/foundation_util.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_internal_cell.h" +#include "testing/gtest/include/gtest/gtest.h" +#import "third_party/ocmock/OCMock/OCMock.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace { + +ContentSuggestionsFaviconInternalCell* GetInnerCell( + NSInteger cellIndex, + ContentSuggestionsFaviconCell* outerCell) { + UICollectionViewCell* innerCell = [outerCell.collectionView.dataSource + collectionView:outerCell.collectionView + cellForItemAtIndexPath:[NSIndexPath indexPathForItem:cellIndex + inSection:0]]; + + EXPECT_EQ([ContentSuggestionsFaviconInternalCell class], [innerCell class]); + return base::mac::ObjCCastStrict<ContentSuggestionsFaviconInternalCell>( + innerCell); +} + +#pragma mark - Tests + +// Tests that configureCell: set all the fields of the cell. +TEST(ContentSuggestionsFaviconItemTest, CellIsConfigured) { + id delegateMock = [OCMockObject + mockForProtocol:@protocol(ContentSuggestionsFaviconCellDelegate)]; + ContentSuggestionsFaviconItem* item = + [[ContentSuggestionsFaviconItem alloc] initWithType:0]; + item.delegate = delegateMock; + + ContentSuggestionsFaviconCell* cell = [[[item cellClass] alloc] init]; + ASSERT_EQ([ContentSuggestionsFaviconCell class], [cell class]); + + [item configureCell:cell]; + EXPECT_EQ(delegateMock, cell.delegate); +} + +// Tests that the favicon added to the item are correctly passed to the inner +// collection view. +TEST(ContentSuggestionsFaviconItemTest, AddAFavicon) { + ContentSuggestionsFaviconItem* item = + [[ContentSuggestionsFaviconItem alloc] initWithType:0]; + UIImage* image1 = [[UIImage alloc] init]; + NSString* title1 = @"testTitle1"; + UIImage* image2 = [[UIImage alloc] init]; + NSString* title2 = @"testTitle2"; + [item addFavicon:image1 withTitle:title1]; + [item addFavicon:image2 withTitle:title2]; + + ContentSuggestionsFaviconCell* cell = [[[item cellClass] alloc] init]; + + [item configureCell:cell]; + + ContentSuggestionsFaviconInternalCell* faviconInternalCell1 = + GetInnerCell(0, cell); + EXPECT_EQ(image1, faviconInternalCell1.faviconView.image); + EXPECT_TRUE([title1 isEqualToString:faviconInternalCell1.titleLabel.text]); + + ContentSuggestionsFaviconInternalCell* faviconInternalCell2 = + GetInnerCell(1, cell); + EXPECT_EQ(image2, faviconInternalCell2.faviconView.image); + EXPECT_TRUE([title2 isEqualToString:faviconInternalCell2.titleLabel.text]); +} + +} // namespace
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_item.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_item.h similarity index 72% rename from ios/chrome/browser/ui/suggestions/suggestions_item.h rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_item.h index c75690f..87b27e3 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_item.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_item.h
@@ -2,14 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_ITEM_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_ITEM_H_ +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_ITEM_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_ITEM_H_ #import "ios/chrome/browser/ui/collection_view/cells/collection_view_item.h" #import "ios/third_party/material_components_ios/src/components/CollectionCells/src/MaterialCollectionCells.h" // Item for a suggestions item with a title and subtitle. -@interface SuggestionsItem : CollectionViewItem +@interface ContentSuggestionsItem : CollectionViewItem // Init a suggestions item with a |title| and a |subtitle|. |type| is the type // of the item. @@ -21,11 +21,11 @@ @end // Corresponding cell for a suggestion item. -@interface SuggestionsCell : MDCCollectionViewCell +@interface ContentSuggestionsCell : MDCCollectionViewCell @property(nonatomic, readonly, strong) UIButton* titleButton; @property(nonatomic, readonly, strong) UILabel* detailTextLabel; @end -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_ITEM_H_ +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_ITEM_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_item.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_item.mm similarity index 86% rename from ios/chrome/browser/ui/suggestions/suggestions_item.mm rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_item.mm index a19986c8..abc84257 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_item.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_item.mm
@@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#import "ios/chrome/browser/ui/suggestions/suggestions_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_item.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_item_actions.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_item_actions.h" #import "ios/third_party/material_components_ios/src/components/Palettes/src/MaterialPalettes.h" #import "ios/third_party/material_components_ios/src/components/Typography/src/MaterialTypography.h" @@ -12,14 +12,14 @@ #error "This file requires ARC support." #endif -@interface SuggestionsItem () +@interface ContentSuggestionsItem () @property(nonatomic, copy) NSString* title; @property(nonatomic, copy) NSString* subtitle; @end -@implementation SuggestionsItem +@implementation ContentSuggestionsItem @synthesize title = _title; @synthesize subtitle = _subtitle; @@ -29,7 +29,7 @@ subtitle:(NSString*)subtitle { self = [super initWithType:type]; if (self) { - self.cellClass = [SuggestionsCell class]; + self.cellClass = [ContentSuggestionsCell class]; _title = [title copy]; _subtitle = [subtitle copy]; } @@ -38,7 +38,7 @@ #pragma mark - CollectionViewItem -- (void)configureCell:(SuggestionsCell*)cell { +- (void)configureCell:(ContentSuggestionsCell*)cell { [super configureCell:cell]; [cell.titleButton setTitle:self.title forState:UIControlStateNormal]; cell.detailTextLabel.text = self.subtitle; @@ -46,9 +46,9 @@ @end -#pragma mark - SuggestionsCell +#pragma mark - ContentSuggestionsCell -@implementation SuggestionsCell +@implementation ContentSuggestionsCell @synthesize titleButton = _titleButton; @synthesize detailTextLabel = _detailTextLabel;
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_item_actions.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_item_actions.h similarity index 61% rename from ios/chrome/browser/ui/suggestions/suggestions_item_actions.h rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_item_actions.h index b3b0301..0d1a80cd 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_item_actions.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_item_actions.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_ITEM_ACTIONS_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_ITEM_ACTIONS_H_ +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_ITEM_ACTIONS_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_ITEM_ACTIONS_H_ // Protocol handling the actions sent in the responder chain by the suggestions // items. @@ -15,4 +15,4 @@ @end -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_ITEM_ACTIONS_H_ +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_ITEM_ACTIONS_H_
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_item_unittest.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_item_unittest.mm new file mode 100644 index 0000000..dbac1dcb --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_item_unittest.mm
@@ -0,0 +1,31 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_item.h" + +#include "testing/gtest/include/gtest/gtest.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace { + +// Tests that configureCell: set all the fields of the cell. +TEST(ContentSuggestionsItemTest, CellIsConfigured) { + NSString* title = @"testTitle"; + NSString* subtitle = @"testSubtitle"; + ContentSuggestionsItem* item = + [[ContentSuggestionsItem alloc] initWithType:0 + title:title + subtitle:subtitle]; + ContentSuggestionsCell* cell = [[[item cellClass] alloc] init]; + EXPECT_EQ([ContentSuggestionsCell class], [cell class]); + + [item configureCell:cell]; + EXPECT_EQ(title, [cell.titleButton titleForState:UIControlStateNormal]); + EXPECT_EQ(subtitle, cell.detailTextLabel.text); +} + +} // namespace
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_stack_item.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item.h similarity index 74% rename from ios/chrome/browser/ui/suggestions/suggestions_stack_item.h rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item.h index 3dafe83..93a8313 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_stack_item.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_STACK_ITEM_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_STACK_ITEM_H_ +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_STACK_ITEM_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_STACK_ITEM_H_ #import "ios/chrome/browser/ui/collection_view/cells/collection_view_item.h" #import "ios/third_party/material_components_ios/src/components/CollectionCells/src/MaterialCollectionCells.h" @@ -11,7 +11,7 @@ // Item for a suggestions stack item. The stack items show an item on the top // and hint that there are other items available. Click on the top item will // trigger an action, clicking the rest of the cell another. -@interface SuggestionsStackItem : CollectionViewItem +@interface ContentSuggestionsStackItem : CollectionViewItem // Initialize a suggestions item with a |title| and a |subtitle|. |type| is the // type of the item. @@ -23,11 +23,11 @@ @end // Corresponding cell for a suggestions stack item. -@interface SuggestionsStackCell : MDCCollectionViewCell +@interface ContentSuggestionsStackCell : MDCCollectionViewCell @property(nonatomic, readonly, strong) UIButton* titleButton; @property(nonatomic, readonly, strong) UILabel* detailTextLabel; @end -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_STACK_ITEM_H_ +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_STACK_ITEM_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_stack_item.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item.mm similarity index 92% rename from ios/chrome/browser/ui/suggestions/suggestions_stack_item.mm rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item.mm index 1ea97d7..b7ab5a2 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_stack_item.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item.mm
@@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#import "ios/chrome/browser/ui/suggestions/suggestions_stack_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_stack_item_actions.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item_actions.h" #import "ios/chrome/browser/ui/uikit_ui_util.h" #if !defined(__has_feature) || !__has_feature(objc_arc) @@ -15,7 +15,7 @@ const CGFloat kStackSpacing = 8; } -@implementation SuggestionsStackItem { +@implementation ContentSuggestionsStackItem { NSString* _title; NSString* _subtitle; } @@ -25,7 +25,7 @@ subtitle:(NSString*)subtitle { self = [super initWithType:type]; if (self) { - self.cellClass = [SuggestionsStackCell class]; + self.cellClass = [ContentSuggestionsStackCell class]; _title = [title copy]; _subtitle = [subtitle copy]; } @@ -34,7 +34,7 @@ #pragma mark - CollectionViewItem -- (void)configureCell:(SuggestionsStackCell*)cell { +- (void)configureCell:(ContentSuggestionsStackCell*)cell { [super configureCell:cell]; [cell.titleButton setTitle:_title forState:UIControlStateNormal]; cell.detailTextLabel.text = _subtitle; @@ -42,7 +42,7 @@ @end -@implementation SuggestionsStackCell +@implementation ContentSuggestionsStackCell @synthesize titleButton = _titleButton; @synthesize detailTextLabel = _detailTextLabel;
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item_actions.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item_actions.h new file mode 100644 index 0000000..fa72be2 --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item_actions.h
@@ -0,0 +1,16 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_STACK_ITEM_ACTIONS_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_STACK_ITEM_ACTIONS_H_ + +// Actions for the Suggestions Stack. +@protocol SuggestionsStackItemActions + +// The item on the top of the stack have been pressed. +- (void)openReadingListFirstItem:(id)sender; + +@end + +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_STACK_ITEM_ACTIONS_H_
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item_unittest.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item_unittest.mm new file mode 100644 index 0000000..6c182cfb --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item_unittest.mm
@@ -0,0 +1,31 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item.h" + +#include "testing/gtest/include/gtest/gtest.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace { + +// Tests that configureCell: set all the fields of the cell. +TEST(ContentSuggestionsStackItemTest, CellIsConfigured) { + NSString* title = @"testTitle"; + NSString* subtitle = @"testSubtitle"; + ContentSuggestionsStackItem* item = + [[ContentSuggestionsStackItem alloc] initWithType:0 + title:title + subtitle:subtitle]; + ContentSuggestionsStackCell* cell = [[[item cellClass] alloc] init]; + EXPECT_EQ([ContentSuggestionsStackCell class], [cell class]); + + [item configureCell:cell]; + EXPECT_EQ(title, [cell.titleButton titleForState:UIControlStateNormal]); + EXPECT_EQ(subtitle, cell.detailTextLabel.text); +} + +} // namespace
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h new file mode 100644 index 0000000..df6a46d --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h
@@ -0,0 +1,34 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_VIEW_CONTROLLER_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_VIEW_CONTROLLER_H_ + +#import <UIKit/UIKit.h> + +#import "ios/chrome/browser/ui/collection_view/collection_view_controller.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.h" + +@protocol ContentSuggestionsCommands; + +// CollectionViewController to display the suggestions items. +@interface ContentSuggestionsViewController + : CollectionViewController<ContentSuggestionsExpandableCellDelegate, + ContentSuggestionsFaviconCellDelegate> + +// Handler for the commands sent by the ContentSuggestionsViewController. +@property(nonatomic, weak) id<ContentSuggestionsCommands> + suggestionCommandHandler; + +// Adds a text item with a |title| and a |subtitle| in the section numbered +// |section|. If |section| is greater than the current number of section, it +// will add a new section at the end. +- (void)addTextItem:(NSString*)title + subtitle:(NSString*)subtitle + toSection:(NSInteger)inputSection; + +@end + +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_VIEW_CONTROLLER_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_view_controller.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm similarity index 76% rename from ios/chrome/browser/ui/suggestions/suggestions_view_controller.mm rename to ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm index adc27da..3b82cd53 100644 --- a/ios/chrome/browser/ui/suggestions/suggestions_view_controller.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm
@@ -2,18 +2,18 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#import "ios/chrome/browser/ui/suggestions/suggestions_view_controller.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h" #include "base/mac/foundation_util.h" #import "ios/chrome/browser/ui/collection_view/cells/MDCCollectionViewCell+Chrome.h" #import "ios/chrome/browser/ui/collection_view/cells/collection_view_item.h" #import "ios/chrome/browser/ui/collection_view/collection_view_model.h" -#import "ios/chrome/browser/ui/suggestions/expandable_item.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_collection_updater.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_commands.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_item_actions.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_stack_item.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_stack_item_actions.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_commands.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_item_actions.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_stack_item_actions.h" +#import "ios/chrome/browser/ui/content_suggestions/expandable_item.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." @@ -23,18 +23,19 @@ const NSTimeInterval kAnimationDuration = 0.35; } // namespace -@interface SuggestionsViewController ()<SuggestionsItemActions, - SuggestionsStackItemActions> +@interface ContentSuggestionsViewController ()<SuggestionsItemActions, + SuggestionsStackItemActions> -@property(nonatomic, strong) SuggestionsCollectionUpdater* collectionUpdater; +@property(nonatomic, strong) + ContentSuggestionsCollectionUpdater* collectionUpdater; -// Expand or collapse the |cell|, if it is a SuggestionsExpandableCell, +// Expand or collapse the |cell|, if it is a ContentSuggestionsExpandableCell, // according to |expand|. - (void)expand:(BOOL)expand cell:(UICollectionViewCell*)cell; @end -@implementation SuggestionsViewController +@implementation ContentSuggestionsViewController @synthesize suggestionCommandHandler = _suggestionCommandHandler; @synthesize collectionUpdater = _collectionUpdater; @@ -44,7 +45,7 @@ - (void)viewDidLoad { [super viewDidLoad]; - _collectionUpdater = [[SuggestionsCollectionUpdater alloc] init]; + _collectionUpdater = [[ContentSuggestionsCollectionUpdater alloc] init]; _collectionUpdater.collectionViewController = self; self.collectionView.delegate = self; @@ -64,7 +65,7 @@ } } -#pragma mark - SuggestionsExpandableCellDelegate +#pragma mark - ContentSuggestionsExpandableCellDelegate - (void)collapseCell:(UICollectionViewCell*)cell { [self expand:NO cell:cell]; @@ -74,7 +75,7 @@ [self expand:YES cell:cell]; } -#pragma mark - SuggestionsFaviconCellDelegate +#pragma mark - ContentSuggestionsFaviconCellDelegate - (void)openFaviconAtIndexPath:(NSIndexPath*)innerIndexPath { [self.suggestionCommandHandler openFaviconAtIndex:innerIndexPath.item]; @@ -86,7 +87,7 @@ [self.suggestionCommandHandler addEmptyItem]; } -#pragma mark - SuggestionsCollectionUpdater forwarding +#pragma mark - ContentSuggestionsCollectionUpdater forwarding - (void)addTextItem:(NSString*)title subtitle:(NSString*)subtitle @@ -142,9 +143,8 @@ NSIndexPath* indexPath = [self.collectionView indexPathForCell:cell]; CollectionViewItem* item = [self.collectionViewModel itemAtIndexPath:indexPath]; - if ([item conformsToProtocol:@protocol(SuggestionsExpandableArticle)]) { - id<SuggestionsExpandableArticle> expandableItem = - (id<SuggestionsExpandableArticle>)item; + if ([item conformsToProtocol:@protocol(ExpandableItem)]) { + id<ExpandableItem> expandableItem = (id<ExpandableItem>)item; NSInteger sectionIdentifier = [self.collectionViewModel sectionIdentifierForSection:indexPath.section];
diff --git a/ios/chrome/browser/ui/content_suggestions/expandable_item.h b/ios/chrome/browser/ui/content_suggestions/expandable_item.h new file mode 100644 index 0000000..caf1699 --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/expandable_item.h
@@ -0,0 +1,18 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_EXPANDABLE_ITEM_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_EXPANDABLE_ITEM_H_ + +#import <UIKit/UIKit.h> + +// Protocol allowing a CollectionViewItem to be expanded. +@protocol ExpandableItem + +// Whether the cells should be in expanded mode. +@property(nonatomic) BOOL expanded; + +@end + +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_EXPANDABLE_ITEM_H_
diff --git a/ios/chrome/browser/ui/suggestions/BUILD.gn b/ios/chrome/browser/ui/suggestions/BUILD.gn deleted file mode 100644 index 0a05622e..0000000 --- a/ios/chrome/browser/ui/suggestions/BUILD.gn +++ /dev/null
@@ -1,58 +0,0 @@ -# Copyright 2016 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -source_set("suggestions") { - configs += [ "//build/config/compiler:enable_arc" ] - sources = [ - "expandable_item.h", - "suggestions_article_item.h", - "suggestions_article_item.mm", - "suggestions_collection_updater.h", - "suggestions_collection_updater.mm", - "suggestions_commands.h", - "suggestions_expandable_item.h", - "suggestions_expandable_item.mm", - "suggestions_favicon_internal_cell.h", - "suggestions_favicon_internal_cell.mm", - "suggestions_favicon_item.h", - "suggestions_favicon_item.mm", - "suggestions_item.h", - "suggestions_item.mm", - "suggestions_item_actions.h", - "suggestions_stack_item.h", - "suggestions_stack_item.mm", - "suggestions_stack_item_actions.h", - "suggestions_view_controller.h", - "suggestions_view_controller.mm", - ] - deps = [ - "//base", - "//ios/chrome/browser/ui", - "//ios/chrome/browser/ui/collection_view", - "//ios/third_party/material_roboto_font_loader_ios", - "//ui/base", - ] - public_deps = [ - "//ios/third_party/material_components_ios", - ] -} - -source_set("unit_tests") { - configs += [ "//build/config/compiler:enable_arc" ] - testonly = true - sources = [ - "suggestions_article_item_unittest.mm", - "suggestions_expandable_item_unittest.mm", - "suggestions_favicon_item_unittest.mm", - "suggestions_item_unittest.mm", - "suggestions_stack_item_unittest.mm", - ] - deps = [ - ":suggestions", - "//base", - "//ios/chrome/browser/ui/collection_view", - "//testing/gtest", - "//third_party/ocmock", - ] -}
diff --git a/ios/chrome/browser/ui/suggestions/expandable_item.h b/ios/chrome/browser/ui/suggestions/expandable_item.h deleted file mode 100644 index b96c112..0000000 --- a/ios/chrome/browser/ui/suggestions/expandable_item.h +++ /dev/null
@@ -1,18 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_EXPANDABLE_ITEM_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_EXPANDABLE_ITEM_H_ - -#import <UIKit/UIKit.h> - -// Protocol allowing a CollectionViewItem to be expanded. -@protocol SuggestionsExpandableArticle - -// Whether the cells should be in expanded mode. -@property(nonatomic) BOOL expanded; - -@end - -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_EXPANDABLE_ITEM_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_article_item_unittest.mm b/ios/chrome/browser/ui/suggestions/suggestions_article_item_unittest.mm deleted file mode 100644 index aa20975..0000000 --- a/ios/chrome/browser/ui/suggestions/suggestions_article_item_unittest.mm +++ /dev/null
@@ -1,34 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "ios/chrome/browser/ui/suggestions/suggestions_article_item.h" - -#include "testing/gtest/include/gtest/gtest.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -namespace { - -// Tests that configureCell: set all the fields of the cell. -TEST(SuggestionsArticleItemTest, CellIsConfigured) { - NSString* title = @"testTitle"; - NSString* subtitle = @"testSubtitle"; - UIImage* image = [[UIImage alloc] init]; - SuggestionsArticleItem* item = - [[SuggestionsArticleItem alloc] initWithType:0 - title:title - subtitle:subtitle - image:image]; - SuggestionsArticleCell* cell = [[[item cellClass] alloc] init]; - EXPECT_TRUE([cell isMemberOfClass:[SuggestionsArticleCell class]]); - - [item configureCell:cell]; - EXPECT_EQ(title, cell.titleLabel.text); - EXPECT_EQ(subtitle, cell.subtitleLabel.text); - EXPECT_EQ(image, cell.imageView.image); -} - -} // namespace
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_commands.h b/ios/chrome/browser/ui/suggestions/suggestions_commands.h deleted file mode 100644 index 501b497..0000000 --- a/ios/chrome/browser/ui/suggestions/suggestions_commands.h +++ /dev/null
@@ -1,22 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_COMMANDS_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_COMMANDS_H_ - -// Commands protocol for the SuggestionsViewController. -@protocol SuggestionsCommands - -// Adds a new empty SuggestionItem. -- (void)addEmptyItem; -// Opens the Reading List. -- (void)openReadingList; -// Opens the first page of the Reading List. -- (void)openFirstPageOfReadingList; -// Opens the favicon associated with the cell with the |index|. -- (void)openFaviconAtIndex:(NSInteger)index; - -@end - -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_COMMANDS_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_favicon_internal_cell.h b/ios/chrome/browser/ui/suggestions/suggestions_favicon_internal_cell.h deleted file mode 100644 index ab45c1ef..0000000 --- a/ios/chrome/browser/ui/suggestions/suggestions_favicon_internal_cell.h +++ /dev/null
@@ -1,20 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_FAVICON_INTERNAL_CELL_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_FAVICON_INTERNAL_CELL_H_ - -#import <UIKit/UIKit.h> - -// Cell to display a favicon in the internal collection view. -@interface SuggestionsFaviconInternalCell : UICollectionViewCell - -@property(nonatomic, strong) UIImageView* faviconView; -@property(nonatomic, strong) UILabel* titleLabel; - -+ (NSString*)reuseIdentifier; - -@end - -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_FAVICON_INTERNAL_CELL_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_favicon_item.h b/ios/chrome/browser/ui/suggestions/suggestions_favicon_item.h deleted file mode 100644 index 235944e..0000000 --- a/ios/chrome/browser/ui/suggestions/suggestions_favicon_item.h +++ /dev/null
@@ -1,42 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_FAVICON_ITEM_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_FAVICON_ITEM_H_ - -#import "ios/chrome/browser/ui/collection_view/cells/collection_view_item.h" -#import "ios/third_party/material_components_ios/src/components/CollectionCells/src/MaterialCollectionCells.h" - -// Delegate for the SuggestionsFaviconCell, handling the interaction with the -// inner collection view. -@protocol SuggestionsFaviconCellDelegate - -// Opens the favicon associated with the |indexPath| of the internal cell. -- (void)openFaviconAtIndexPath:(NSIndexPath*)innerIndexPath; - -@end - -// Item for a cell containing favicons in the suggestions. The favicons are -// presented in an inner collection view contained in the cell associated with -// this item. The collection view scrolls horizontally. -@interface SuggestionsFaviconItem : CollectionViewItem - -@property(nonatomic, weak) id<SuggestionsFaviconCellDelegate> delegate; - -// Adds a favicon with a |favicon| image and a |title| to this item. -- (void)addFavicon:(UIImage*)favicon withTitle:(NSString*)title; - -@end - -// The corresponding cell of a favicon item. -@interface SuggestionsFaviconCell : MDCCollectionViewCell - -// The inner collection view, used to display the favicons. -@property(nonatomic, strong) UICollectionView* collectionView; - -@property(nonatomic, weak) id<SuggestionsFaviconCellDelegate> delegate; - -@end - -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_FAVICON_ITEM_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_favicon_item_unittest.mm b/ios/chrome/browser/ui/suggestions/suggestions_favicon_item_unittest.mm deleted file mode 100644 index 494dfe4..0000000 --- a/ios/chrome/browser/ui/suggestions/suggestions_favicon_item_unittest.mm +++ /dev/null
@@ -1,72 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "ios/chrome/browser/ui/suggestions/suggestions_favicon_item.h" - -#include "base/mac/foundation_util.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_favicon_internal_cell.h" -#include "testing/gtest/include/gtest/gtest.h" -#import "third_party/ocmock/OCMock/OCMock.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -namespace { - -SuggestionsFaviconInternalCell* GetInnerCell( - NSInteger cellIndex, - SuggestionsFaviconCell* outerCell) { - UICollectionViewCell* innerCell = [outerCell.collectionView.dataSource - collectionView:outerCell.collectionView - cellForItemAtIndexPath:[NSIndexPath indexPathForItem:cellIndex - inSection:0]]; - - EXPECT_EQ([SuggestionsFaviconInternalCell class], [innerCell class]); - return base::mac::ObjCCastStrict<SuggestionsFaviconInternalCell>(innerCell); -} - -#pragma mark - Tests - -// Tests that configureCell: set all the fields of the cell. -TEST(SuggestionsFaviconItemTest, CellIsConfigured) { - id delegateMock = - [OCMockObject mockForProtocol:@protocol(SuggestionsFaviconCellDelegate)]; - SuggestionsFaviconItem* item = - [[SuggestionsFaviconItem alloc] initWithType:0]; - item.delegate = delegateMock; - - SuggestionsFaviconCell* cell = [[[item cellClass] alloc] init]; - ASSERT_EQ([SuggestionsFaviconCell class], [cell class]); - - [item configureCell:cell]; - EXPECT_EQ(delegateMock, cell.delegate); -} - -// Tests that the favicon added to the item are correctly passed to the inner -// collection view. -TEST(SuggestionsFaviconItemTest, AddAFavicon) { - SuggestionsFaviconItem* item = - [[SuggestionsFaviconItem alloc] initWithType:0]; - UIImage* image1 = [[UIImage alloc] init]; - NSString* title1 = @"testTitle1"; - UIImage* image2 = [[UIImage alloc] init]; - NSString* title2 = @"testTitle2"; - [item addFavicon:image1 withTitle:title1]; - [item addFavicon:image2 withTitle:title2]; - - SuggestionsFaviconCell* cell = [[[item cellClass] alloc] init]; - - [item configureCell:cell]; - - SuggestionsFaviconInternalCell* faviconInternalCell1 = GetInnerCell(0, cell); - EXPECT_EQ(image1, faviconInternalCell1.faviconView.image); - EXPECT_TRUE([title1 isEqualToString:faviconInternalCell1.titleLabel.text]); - - SuggestionsFaviconInternalCell* faviconInternalCell2 = GetInnerCell(1, cell); - EXPECT_EQ(image2, faviconInternalCell2.faviconView.image); - EXPECT_TRUE([title2 isEqualToString:faviconInternalCell2.titleLabel.text]); -} - -} // namespace
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_item_unittest.mm b/ios/chrome/browser/ui/suggestions/suggestions_item_unittest.mm deleted file mode 100644 index 7c6f344..0000000 --- a/ios/chrome/browser/ui/suggestions/suggestions_item_unittest.mm +++ /dev/null
@@ -1,29 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "ios/chrome/browser/ui/suggestions/suggestions_item.h" - -#include "testing/gtest/include/gtest/gtest.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -namespace { - -// Tests that configureCell: set all the fields of the cell. -TEST(SuggestionsItemTest, CellIsConfigured) { - NSString* title = @"testTitle"; - NSString* subtitle = @"testSubtitle"; - SuggestionsItem* item = - [[SuggestionsItem alloc] initWithType:0 title:title subtitle:subtitle]; - SuggestionsCell* cell = [[[item cellClass] alloc] init]; - EXPECT_TRUE([cell isMemberOfClass:[SuggestionsCell class]]); - - [item configureCell:cell]; - EXPECT_EQ(title, [cell.titleButton titleForState:UIControlStateNormal]); - EXPECT_EQ(subtitle, cell.detailTextLabel.text); -} - -} // namespace
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_stack_item_actions.h b/ios/chrome/browser/ui/suggestions/suggestions_stack_item_actions.h deleted file mode 100644 index 6a87c19f..0000000 --- a/ios/chrome/browser/ui/suggestions/suggestions_stack_item_actions.h +++ /dev/null
@@ -1,16 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_STACK_ITEM_ACTIONS_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_STACK_ITEM_ACTIONS_H_ - -// Actions for the Suggestions Stack. -@protocol SuggestionsStackItemActions - -// The item on the top of the stack have been pressed. -- (void)openReadingListFirstItem:(id)sender; - -@end - -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_STACK_ITEM_ACTIONS_H_
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_stack_item_unittest.mm b/ios/chrome/browser/ui/suggestions/suggestions_stack_item_unittest.mm deleted file mode 100644 index 4a6ddec4..0000000 --- a/ios/chrome/browser/ui/suggestions/suggestions_stack_item_unittest.mm +++ /dev/null
@@ -1,31 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "ios/chrome/browser/ui/suggestions/suggestions_stack_item.h" - -#include "testing/gtest/include/gtest/gtest.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -namespace { - -// Tests that configureCell: set all the fields of the cell. -TEST(SuggestionsStackItemTest, CellIsConfigured) { - NSString* title = @"testTitle"; - NSString* subtitle = @"testSubtitle"; - SuggestionsStackItem* item = - [[SuggestionsStackItem alloc] initWithType:0 - title:title - subtitle:subtitle]; - SuggestionsStackCell* cell = [[[item cellClass] alloc] init]; - EXPECT_TRUE([cell isMemberOfClass:[SuggestionsStackCell class]]); - - [item configureCell:cell]; - EXPECT_EQ(title, [cell.titleButton titleForState:UIControlStateNormal]); - EXPECT_EQ(subtitle, cell.detailTextLabel.text); -} - -} // namespace
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_view_controller.h b/ios/chrome/browser/ui/suggestions/suggestions_view_controller.h deleted file mode 100644 index 0a23e3f..0000000 --- a/ios/chrome/browser/ui/suggestions/suggestions_view_controller.h +++ /dev/null
@@ -1,33 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_VIEW_CONTROLLER_H_ -#define IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_VIEW_CONTROLLER_H_ - -#import <UIKit/UIKit.h> - -#import "ios/chrome/browser/ui/collection_view/collection_view_controller.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_expandable_item.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_favicon_item.h" - -@protocol SuggestionsCommands; - -// CollectionViewController to display the suggestions items. -@interface SuggestionsViewController - : CollectionViewController<SuggestionsExpandableCellDelegate, - SuggestionsFaviconCellDelegate> - -// Handler for the commands sent by the SuggestionsViewController. -@property(nonatomic, weak) id<SuggestionsCommands> suggestionCommandHandler; - -// Adds a text item with a |title| and a |subtitle| in the section numbered -// |section|. If |section| is greater than the current number of section, it -// will add a new section at the end. -- (void)addTextItem:(NSString*)title - subtitle:(NSString*)subtitle - toSection:(NSInteger)inputSection; - -@end - -#endif // IOS_CHROME_BROWSER_UI_SUGGESTIONS_SUGGESTIONS_VIEW_CONTROLLER_H_
diff --git a/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.h b/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.h index 2ce699d..d1f4d50 100644 --- a/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.h +++ b/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.h
@@ -31,6 +31,7 @@ extern NSString* const kToolsMenuRequestDesktopId; extern NSString* const kToolsMenuSettingsId; extern NSString* const kToolsMenuHelpId; +extern NSString* const kToolsMenuSuggestionsId; // Tools Popup Table Delegate Protocol @protocol ToolsPopupTableDelegate<NSObject>
diff --git a/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.mm b/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.mm index e01c61f9..f05adfde 100644 --- a/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.mm +++ b/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.mm
@@ -52,6 +52,7 @@ NSString* const kToolsMenuRequestDesktopId = @"kToolsMenuRequestDesktopId"; NSString* const kToolsMenuSettingsId = @"kToolsMenuSettingsId"; NSString* const kToolsMenuHelpId = @"kToolsMenuHelpId"; +NSString* const kToolsMenuSuggestionsId = @"kToolsMenuSuggestionsId"; namespace { @@ -119,6 +120,9 @@ { IDS_IOS_TOOLS_MENU_READING_LIST, kToolsMenuReadingListId, IDC_SHOW_READING_LIST, kToolbarTypeWebAll, 0, [ReadingListMenuViewItem class] }, + { IDS_IOS_TOOLS_MENU_SUGGESTIONS, kToolsMenuSuggestionsId, + IDC_SHOW_SUGGESTIONS, kToolbarTypeWebAll, + 0, nil }, { IDS_IOS_TOOLS_MENU_RECENT_TABS, kToolsMenuOtherDevicesId, IDC_SHOW_OTHER_DEVICES, kToolbarTypeWebAll, kVisibleNotIncognitoOnly, nil }, @@ -176,6 +180,12 @@ } } + if (item.title_id == IDS_IOS_TOOLS_MENU_SUGGESTIONS) { + if (!experimental_flags::IsSuggestionsUIEnabled()) { + return NO; + } + } + if (item.title_id == IDS_IOS_OPTIONS_REPORT_AN_ISSUE) { if (!ios::GetChromeBrowserProvider() ->GetUserFeedbackProvider()
diff --git a/ios/chrome/browser/ui/tools_menu/tools_popup_controller.mm b/ios/chrome/browser/ui/tools_menu/tools_popup_controller.mm index 9a5a274..d549374 100644 --- a/ios/chrome/browser/ui/tools_menu/tools_popup_controller.mm +++ b/ios/chrome/browser/ui/tools_menu/tools_popup_controller.mm
@@ -224,6 +224,9 @@ case IDC_SHOW_READING_LIST: base::RecordAction(UserMetricsAction("MobileMenuReadingList")); break; + case IDC_SHOW_SUGGESTIONS: + // TODO(crbug.com/682174): Move it out of the tool menu or add metrics. + break; default: NOTREACHED(); break;
diff --git a/ios/chrome/test/BUILD.gn b/ios/chrome/test/BUILD.gn index 72560b14..9ae5c50 100644 --- a/ios/chrome/test/BUILD.gn +++ b/ios/chrome/test/BUILD.gn
@@ -149,6 +149,7 @@ "//ios/chrome/browser/ui/collection_view:unit_tests", "//ios/chrome/browser/ui/collection_view/cells:unit_tests", "//ios/chrome/browser/ui/commands:unit_tests", + "//ios/chrome/browser/ui/content_suggestions:unit_tests", "//ios/chrome/browser/ui/context_menu:unit_tests", "//ios/chrome/browser/ui/contextual_search:unit_tests", "//ios/chrome/browser/ui/dialogs:unit_tests", @@ -172,7 +173,6 @@ "//ios/chrome/browser/ui/side_swipe:unit_tests", "//ios/chrome/browser/ui/stack_view:unit_tests", "//ios/chrome/browser/ui/static_content:unit_tests", - "//ios/chrome/browser/ui/suggestions:unit_tests", "//ios/chrome/browser/ui/tab_switcher:unit_tests", "//ios/chrome/browser/ui/tabs:unit_tests", "//ios/chrome/browser/ui/toolbar:unit_tests",
diff --git a/ios/clean/chrome/browser/BUILD.gn b/ios/clean/chrome/browser/BUILD.gn index 5bd2dcb..621ee11 100644 --- a/ios/clean/chrome/browser/BUILD.gn +++ b/ios/clean/chrome/browser/BUILD.gn
@@ -35,6 +35,7 @@ "//base", "//base/test:test_support", "//ios/chrome/test/base", + "//ios/shared/chrome/browser/coordinator_context", "//testing/gtest", ] }
diff --git a/ios/clean/chrome/browser/browser_coordinator.h b/ios/clean/chrome/browser/browser_coordinator.h index efd836e0..d28987e6 100644 --- a/ios/clean/chrome/browser/browser_coordinator.h +++ b/ios/clean/chrome/browser/browser_coordinator.h
@@ -31,12 +31,6 @@ // doesn't transfer ownership of the browser state. @property(nonatomic, assign) ios::ChromeBrowserState* browserState; -// The view controller that this coordinator will use to present its content, if -// it is presenting content. This is not the view controller created and managed -// by this coordinator; it should be supplied by whatever object is creating -// this coordinator. -@property(nonatomic, weak) UIViewController* baseViewController; - // The basic lifecycle methods for coordinators are -start and -stop. These // are blank template methods; child classes are expected to implement them and // do not need to invoke the superclass methods.
diff --git a/ios/clean/chrome/browser/browser_coordinator.mm b/ios/clean/chrome/browser/browser_coordinator.mm index c2c7372..950cd93e 100644 --- a/ios/clean/chrome/browser/browser_coordinator.mm +++ b/ios/clean/chrome/browser/browser_coordinator.mm
@@ -28,7 +28,6 @@ @synthesize context = _context; @synthesize browserState = _browserState; -@synthesize baseViewController = _baseViewController; @synthesize childCoordinators = _childCoordinators; @synthesize parentCoordinator = _parentCoordinator; @synthesize overlaying = _overlaying; @@ -72,7 +71,7 @@ [self.childCoordinators addObject:coordinator]; coordinator.parentCoordinator = self; coordinator.browserState = self.browserState; - coordinator.baseViewController = self.viewController; + coordinator.context.baseViewController = self.viewController; } - (BrowserCoordinator*)overlayCoordinator {
diff --git a/ios/clean/chrome/browser/browser_coordinator_unittest.mm b/ios/clean/chrome/browser/browser_coordinator_unittest.mm index 38cb50d..c4fabd2 100644 --- a/ios/clean/chrome/browser/browser_coordinator_unittest.mm +++ b/ios/clean/chrome/browser/browser_coordinator_unittest.mm
@@ -2,9 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#import "ios/clean/chrome/browser/browser_coordinator.h" #import "ios/clean/chrome/browser/browser_coordinator+internal.h" +#import "ios/clean/chrome/browser/browser_coordinator.h" +#import "ios/shared/chrome/browser/coordinator_context/coordinator_context.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest_mac.h" @@ -69,13 +70,13 @@ [parent addChildCoordinator:child]; EXPECT_TRUE([parent.children containsObject:child]); EXPECT_EQ(parent, child.parentCoordinator); - EXPECT_EQ(parent.viewController, child.baseViewController); + EXPECT_EQ(parent.viewController, child.context.baseViewController); [parent removeChildCoordinator:child]; EXPECT_FALSE([parent.children containsObject:child]); EXPECT_EQ(nil, child.parentCoordinator); // Unparenting shouldn't change a child's baseViewController. - EXPECT_EQ(parent.viewController, child.baseViewController); + EXPECT_EQ(parent.viewController, child.context.baseViewController); TestCoordinator* otherParent = [[TestCoordinator alloc] init]; TestCoordinator* otherChild = [[TestCoordinator alloc] init];
diff --git a/ios/clean/chrome/browser/ui/settings/settings_coordinator.mm b/ios/clean/chrome/browser/ui/settings/settings_coordinator.mm index e3dc1423..5edbd3b 100644 --- a/ios/clean/chrome/browser/ui/settings/settings_coordinator.mm +++ b/ios/clean/chrome/browser/ui/settings/settings_coordinator.mm
@@ -32,9 +32,9 @@ newSettingsMainControllerWithMainBrowserState:self.browserState currentBrowserState:self.browserState delegate:self]; - [self.baseViewController presentViewController:self.viewController - animated:self.context.animated - completion:nil]; + [self.context.baseViewController presentViewController:self.viewController + animated:self.context.animated + completion:nil]; } - (void)stop {
diff --git a/ios/clean/chrome/browser/ui/tab/tab_coordinator.mm b/ios/clean/chrome/browser/ui/tab/tab_coordinator.mm index a322190..c449b47 100644 --- a/ios/clean/chrome/browser/ui/tab/tab_coordinator.mm +++ b/ios/clean/chrome/browser/ui/tab/tab_coordinator.mm
@@ -53,7 +53,7 @@ [self addChildCoordinator:webCoordinator]; // Unset the base view controller, so |webCoordinator| doesn't present its // view controller. - webCoordinator.baseViewController = nil; + webCoordinator.context.baseViewController = nil; [webCoordinator start]; ToolbarCoordinator* toolbarCoordinator = [[ToolbarCoordinator alloc] init]; @@ -64,15 +64,15 @@ self.webMediator.webState, toolbarCoordinator); // Unset the base view controller, so |toolbarCoordinator| doesn't present // its view controller. - toolbarCoordinator.baseViewController = nil; + toolbarCoordinator.context.baseViewController = nil; [toolbarCoordinator start]; self.viewController.toolbarViewController = toolbarCoordinator.viewController; self.viewController.contentViewController = webCoordinator.viewController; - [self.baseViewController presentViewController:self.viewController - animated:self.context.animated - completion:nil]; + [self.context.baseViewController presentViewController:self.viewController + animated:self.context.animated + completion:nil]; } - (void)stop {
diff --git a/ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm b/ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm index dc2e855..f91f66e 100644 --- a/ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm +++ b/ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm
@@ -66,9 +66,9 @@ // |baseViewController| is nullable, so this is by design a no-op if it hasn't // been set. This may be true in a unit test, or if this coordinator is being // used as a root coordinator. - [self.baseViewController presentViewController:self.viewController - animated:self.context.animated - completion:nil]; + [self.context.baseViewController presentViewController:self.viewController + animated:self.context.animated + completion:nil]; } #pragma mark - TabGridDataSource
diff --git a/ios/clean/chrome/browser/ui/tab_strip/tab_strip_container_coordinator.mm b/ios/clean/chrome/browser/ui/tab_strip/tab_strip_container_coordinator.mm index f2d66176..47756416 100644 --- a/ios/clean/chrome/browser/ui/tab_strip/tab_strip_container_coordinator.mm +++ b/ios/clean/chrome/browser/ui/tab_strip/tab_strip_container_coordinator.mm
@@ -43,7 +43,7 @@ [self addChildCoordinator:tabCoordinator]; // Unset the base view controller, so |tabCoordinator| doesn't present // its view controller. - tabCoordinator.baseViewController = nil; + tabCoordinator.context.baseViewController = nil; [tabCoordinator start]; // PLACEHOLDER: Replace this placeholder with an actual tab strip view @@ -61,9 +61,9 @@ self.viewController.tabStripViewController = tabStripViewController; self.viewController.contentViewController = tabCoordinator.viewController; - [self.baseViewController presentViewController:self.viewController - animated:self.context.animated - completion:nil]; + [self.context.baseViewController presentViewController:self.viewController + animated:self.context.animated + completion:nil]; } - (void)stop {
diff --git a/ios/clean/chrome/browser/ui/toolbar/toolbar_coordinator.mm b/ios/clean/chrome/browser/ui/toolbar/toolbar_coordinator.mm index 229c50d1..fa452cb1 100644 --- a/ios/clean/chrome/browser/ui/toolbar/toolbar_coordinator.mm +++ b/ios/clean/chrome/browser/ui/toolbar/toolbar_coordinator.mm
@@ -33,9 +33,9 @@ self.viewController = [[ToolbarViewController alloc] init]; self.viewController.toolbarCommandHandler = self; - [self.baseViewController presentViewController:self.viewController - animated:self.context.animated - completion:nil]; + [self.context.baseViewController presentViewController:self.viewController + animated:self.context.animated + completion:nil]; } #pragma mark - CRWWebStateObserver
diff --git a/ios/clean/chrome/browser/ui/tools/tools_coordinator.mm b/ios/clean/chrome/browser/ui/tools/tools_coordinator.mm index d139b87..d80614ef 100644 --- a/ios/clean/chrome/browser/ui/tools/tools_coordinator.mm +++ b/ios/clean/chrome/browser/ui/tools/tools_coordinator.mm
@@ -32,9 +32,9 @@ self.menuViewController.modalPresentationStyle = UIModalPresentationCustom; self.menuViewController.transitioningDelegate = self; - [self.baseViewController presentViewController:self.menuViewController - animated:self.context.animated - completion:nil]; + [self.context.baseViewController presentViewController:self.menuViewController + animated:self.context.animated + completion:nil]; } - (void)stop {
diff --git a/ios/clean/chrome/browser/ui/web_contents/web_coordinator.mm b/ios/clean/chrome/browser/ui/web_contents/web_coordinator.mm index 265a4ee..88d9210 100644 --- a/ios/clean/chrome/browser/ui/web_contents/web_coordinator.mm +++ b/ios/clean/chrome/browser/ui/web_contents/web_coordinator.mm
@@ -34,9 +34,9 @@ // Reminder: this is a no-op if |baseViewController| is nil, for example // when this coordinator's view controller will be contained instead of // presented. - [self.baseViewController presentViewController:self.viewController - animated:self.context.animated - completion:nil]; + [self.context.baseViewController presentViewController:self.viewController + animated:self.context.animated + completion:nil]; } @end
diff --git a/ios/shared/chrome/browser/coordinator_context/coordinator_context.h b/ios/shared/chrome/browser/coordinator_context/coordinator_context.h index f52a62c..3bb3419 100644 --- a/ios/shared/chrome/browser/coordinator_context/coordinator_context.h +++ b/ios/shared/chrome/browser/coordinator_context/coordinator_context.h
@@ -7,8 +7,15 @@ #import <UIKit/UIKit.h> +// Holds data for the coordinator that uses it. @interface CoordinatorContext : NSObject +// The view controller that the coordinator will use to present its content, if +// it is presenting content. This is not the view controller created and managed +// by the coordinator; it should be supplied by whatever object is creating +// the coordinator. +@property(nonatomic, weak) UIViewController* baseViewController; + // Default is YES. @property(nonatomic, assign, getter=isAnimated) BOOL animated;
diff --git a/ios/shared/chrome/browser/coordinator_context/coordinator_context.mm b/ios/shared/chrome/browser/coordinator_context/coordinator_context.mm index f53d002..2815174 100644 --- a/ios/shared/chrome/browser/coordinator_context/coordinator_context.mm +++ b/ios/shared/chrome/browser/coordinator_context/coordinator_context.mm
@@ -10,6 +10,7 @@ @implementation CoordinatorContext +@synthesize baseViewController = _baseViewController; @synthesize animated = _animated; - (instancetype)init {
diff --git a/ios/showcase/core/showcase_model.mm b/ios/showcase/core/showcase_model.mm index d9a87d5a..8576ae91 100644 --- a/ios/showcase/core/showcase_model.mm +++ b/ios/showcase/core/showcase_model.mm
@@ -22,7 +22,7 @@ showcase::kUseCaseKey : @"Main settings screen", }, @{ - showcase::kClassForDisplayKey : @"SuggestionsViewController", + showcase::kClassForDisplayKey : @"ContentSuggestionsViewController", showcase::kClassForInstantiationKey : @"SCSuggestionsCoordinator", showcase::kUseCaseKey : @"New Suggestions UI", },
diff --git a/ios/showcase/suggestions/BUILD.gn b/ios/showcase/suggestions/BUILD.gn index 3bf7c77..1a2c81f 100644 --- a/ios/showcase/suggestions/BUILD.gn +++ b/ios/showcase/suggestions/BUILD.gn
@@ -9,7 +9,7 @@ ] deps = [ "//base", - "//ios/chrome/browser/ui/suggestions", + "//ios/chrome/browser/ui/content_suggestions", "//ios/showcase/common", ] libs = [ "UIKit.framework" ]
diff --git a/ios/showcase/suggestions/sc_suggestions_coordinator.mm b/ios/showcase/suggestions/sc_suggestions_coordinator.mm index 227d6cb..e014083 100644 --- a/ios/showcase/suggestions/sc_suggestions_coordinator.mm +++ b/ios/showcase/suggestions/sc_suggestions_coordinator.mm
@@ -4,8 +4,8 @@ #import "ios/showcase/suggestions/sc_suggestions_coordinator.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_commands.h" -#import "ios/chrome/browser/ui/suggestions/suggestions_view_controller.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_commands.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h" #import "ios/showcase/common/protocol_alerter.h" #if !defined(__has_feature) || !__has_feature(objc_arc) @@ -15,7 +15,7 @@ @interface SCSuggestionsCoordinator () @property(nonatomic, strong) - SuggestionsViewController* suggestionViewController; + ContentSuggestionsViewController* suggestionViewController; @property(nonatomic, strong) ProtocolAlerter* alerter; @end @@ -30,14 +30,14 @@ - (void)start { self.alerter = [[ProtocolAlerter alloc] - initWithProtocols:@[ @protocol(SuggestionsCommands) ]]; + initWithProtocols:@[ @protocol(ContentSuggestionsCommands) ]]; self.alerter.baseViewController = self.baseViewController; - _suggestionViewController = [[SuggestionsViewController alloc] + _suggestionViewController = [[ContentSuggestionsViewController alloc] initWithStyle:CollectionViewControllerStyleDefault]; _suggestionViewController.suggestionCommandHandler = - reinterpret_cast<id<SuggestionsCommands>>(self.alerter); + reinterpret_cast<id<ContentSuggestionsCommands>>(self.alerter); [self.baseViewController pushViewController:_suggestionViewController animated:YES];
diff --git a/ios/showcase/suggestions/sc_suggestions_egtest.mm b/ios/showcase/suggestions/sc_suggestions_egtest.mm index 5a8ec1f..8509dea4 100644 --- a/ios/showcase/suggestions/sc_suggestions_egtest.mm +++ b/ios/showcase/suggestions/sc_suggestions_egtest.mm
@@ -16,9 +16,10 @@ @implementation SCSuggestionsTestCase -// Tests launching TabGridViewController and tapping a cell. +// Tests launching ContentSuggestionsViewController. - (void)testLaunchAndTappingCell { - [[EarlGrey selectElementWithMatcher:grey_text(@"SuggestionsViewController")] + [[EarlGrey + selectElementWithMatcher:grey_text(@"ContentSuggestionsViewController")] performAction:grey_tap()]; }
diff --git a/ios/web/navigation/crw_session_controller.mm b/ios/web/navigation/crw_session_controller.mm index d175790..32fd794 100644 --- a/ios/web/navigation/crw_session_controller.mm +++ b/ios/web/navigation/crw_session_controller.mm
@@ -544,6 +544,7 @@ currentItem->SetURL(url); currentItem->SetSerializedStateObject(stateObject); currentItem->SetHasStateBeenReplaced(true); + currentItem->SetPostData(nil); currentEntry.navigationItem->SetURL(url); // If the change is to a committed entry, notify interested parties. if (currentEntry != self.pendingEntry && _navigationManager)
diff --git a/ios/web/navigation/history_state_operations_inttest.mm b/ios/web/navigation/history_state_operations_inttest.mm index a305cf3..d823e70b 100644 --- a/ios/web/navigation/history_state_operations_inttest.mm +++ b/ios/web/navigation/history_state_operations_inttest.mm
@@ -370,3 +370,25 @@ GetIndexOfNavigationItem(GetLastCommittedItem())); EXPECT_EQ(NSNotFound, GetIndexOfNavigationItem(about_blank_item)); } + +// Tests that performing a replaceState() on a page created with a POST request +// resets the page to a GET request. +TEST_F(HistoryStateOperationsTest, ReplaceStatePostRequest) { + // Add POST data to the current NavigationItem. + base::scoped_nsobject<NSData> post_data([NSData data]); + static_cast<web::NavigationItemImpl*>(GetLastCommittedItem()) + ->SetPostData(post_data); + ASSERT_TRUE(GetLastCommittedItem()->HasPostData()); + // Set up the state parameters and tap the replace state button. + std::string new_state("STATE OBJECT"); + std::string empty_title; + GURL new_url = state_operations_url().Resolve("path"); + SetStateParams(new_state, empty_title, new_url); + ASSERT_TRUE(web::test::TapWebViewElementWithId(web_state(), kReplaceStateId)); + // Verify that url has been replaced. + base::test::ios::WaitUntilCondition(^bool { + return GetLastCommittedItem()->GetURL() == new_url; + }); + // Verify that the NavigationItem no longer has POST data. + EXPECT_FALSE(GetLastCommittedItem()->HasPostData()); +}
diff --git a/testing/buildbot/chromium.linux.json b/testing/buildbot/chromium.linux.json index 3fd2486..2276250 100644 --- a/testing/buildbot/chromium.linux.json +++ b/testing/buildbot/chromium.linux.json
@@ -1210,6 +1210,9 @@ "test": "components_web_restrictions_junit_tests" }, { + "test": "components_variations_junit_tests" + }, + { "test": "content_junit_tests" }, { @@ -2419,6 +2422,9 @@ "test": "components_web_restrictions_junit_tests" }, { + "test": "components_variations_junit_tests" + }, + { "test": "content_junit_tests" }, {
diff --git a/testing/buildbot/gn_isolate_map.pyl b/testing/buildbot/gn_isolate_map.pyl index e6f84ad..03a3d3b 100644 --- a/testing/buildbot/gn_isolate_map.pyl +++ b/testing/buildbot/gn_isolate_map.pyl
@@ -323,6 +323,10 @@ "label": "//components:components_unittests", "type": "windowed_test_launcher", }, + "components_variations_junit_tests": { + "label": "//components/variations/android:components_variations_junit_tests", + "type": "junit_test", + }, "components_web_restrictions_junit_tests": { "label": "//components/web_restrictions:components_web_restrictions_junit_tests", "type": "junit_test",
diff --git a/testing/buildbot/manage.py b/testing/buildbot/manage.py index 95234b9..11f7621a 100755 --- a/testing/buildbot/manage.py +++ b/testing/buildbot/manage.py
@@ -162,6 +162,7 @@ 'chrome_junit_tests', 'components_invalidation_impl_junit_tests', 'components_policy_junit_tests', + 'components_variations_junit_tests', 'components_web_restrictions_junit_tests', 'content_junit_tests', 'content_junit_tests',
diff --git a/third_party/WebKit/LayoutTests/custom-elements/array-squat-crash-expected.txt b/third_party/WebKit/LayoutTests/custom-elements/array-squat-crash-expected.txt new file mode 100644 index 0000000..d7172ab --- /dev/null +++ b/third_party/WebKit/LayoutTests/custom-elements/array-squat-crash-expected.txt
@@ -0,0 +1 @@ +PASS if this does not crash
diff --git a/third_party/WebKit/LayoutTests/custom-elements/array-squat-crash.html b/third_party/WebKit/LayoutTests/custom-elements/array-squat-crash.html new file mode 100644 index 0000000..45a69fc8 --- /dev/null +++ b/third_party/WebKit/LayoutTests/custom-elements/array-squat-crash.html
@@ -0,0 +1,21 @@ +<!DOCTYPE html> +PASS if this does not crash +<script> +if (window.testRunner) { + testRunner.dumpAsText(); +} + +class X extends HTMLElement { + connectedCallback() { + } +}; + +Object.defineProperty(Array.prototype, 0, { + set: function(value) { + throw 'barf'; + }, + enumerable: true +}); + +customElements.define('x-x', X); +</script>
diff --git a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/destroy-context-in-onloadstart.html b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/destroy-context-in-onloadstart.html index 54c7f32..51499b0 100644 --- a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/destroy-context-in-onloadstart.html +++ b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/destroy-context-in-onloadstart.html
@@ -13,7 +13,8 @@ xhr.onloadstart = () => { iframe.parentNode.remove(iframe); }; - assert_throws('NetworkError', () => xhr.send()); + // This should not crash the renderer. + xhr.send(); t.done(); }); }, 'Detach iframe in onloadstart callback.');
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp index fbe07df2..42b8087 100644 --- a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp +++ b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
@@ -11,6 +11,7 @@ #include "bindings/core/v8/V8Element.h" #include "bindings/core/v8/V8ErrorHandler.h" #include "bindings/core/v8/V8HiddenValue.h" +#include "bindings/core/v8/V8PrivateProperty.h" #include "bindings/core/v8/V8ScriptRunner.h" #include "bindings/core/v8/V8ThrowException.h" #include "core/dom/ExceptionCode.h" @@ -82,18 +83,20 @@ return static_cast<ScriptCustomElementDefinition*>(definition); } +using SymbolGetter = V8PrivateProperty::Symbol (*)(v8::Isolate*); + template <typename T> -static void keepAlive(v8::Local<v8::Array>& array, - uint32_t index, +static void keepAlive(v8::Local<v8::Object>& object, + SymbolGetter symbolGetter, const v8::Local<T>& value, ScopedPersistent<T>& persistent, ScriptState* scriptState) { if (value.IsEmpty()) return; - array->Set(scriptState->context(), index, value).ToChecked(); - - persistent.set(scriptState->isolate(), value); + v8::Isolate* isolate = scriptState->isolate(); + symbolGetter(isolate).set(scriptState->context(), object, value); + persistent.set(isolate, value); persistent.setPhantom(); } @@ -122,16 +125,18 @@ // We add the callbacks here to keep them alive. We use the name as // the key because it is unique per-registry. - v8::Local<v8::Array> array = v8::Array::New(scriptState->isolate(), 5); - keepAlive(array, 0, connectedCallback, definition->m_connectedCallback, + v8::Local<v8::Object> object = v8::Object::New(scriptState->isolate()); + keepAlive(object, V8PrivateProperty::getCustomElementConnectedCallback, + connectedCallback, definition->m_connectedCallback, scriptState); + keepAlive(object, V8PrivateProperty::getCustomElementDisconnectedCallback, + disconnectedCallback, definition->m_disconnectedCallback, scriptState); - keepAlive(array, 1, disconnectedCallback, definition->m_disconnectedCallback, + keepAlive(object, V8PrivateProperty::getCustomElementAdoptedCallback, + adoptedCallback, definition->m_adoptedCallback, scriptState); + keepAlive(object, V8PrivateProperty::getCustomElementAttributeChangedCallback, + attributeChangedCallback, definition->m_attributeChangedCallback, scriptState); - keepAlive(array, 2, adoptedCallback, definition->m_adoptedCallback, - scriptState); - keepAlive(array, 3, attributeChangedCallback, - definition->m_attributeChangedCallback, scriptState); - map->Set(scriptState->context(), nameValue, array).ToLocalChecked(); + map->Set(scriptState->context(), nameValue, object).ToLocalChecked(); return definition; }
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptPromiseProperty.h b/third_party/WebKit/Source/bindings/core/v8/ScriptPromiseProperty.h index f979119..eadab15 100644 --- a/third_party/WebKit/Source/bindings/core/v8/ScriptPromiseProperty.h +++ b/third_party/WebKit/Source/bindings/core/v8/ScriptPromiseProperty.h
@@ -181,18 +181,6 @@ template <typename HolderType, typename ResolvedType, typename RejectedType> void ScriptPromiseProperty<HolderType, ResolvedType, RejectedType>::trace( Visitor* visitor) { - traceImpl(visitor); -} -template <typename HolderType, typename ResolvedType, typename RejectedType> -void ScriptPromiseProperty<HolderType, ResolvedType, RejectedType>::trace( - InlinedGlobalMarkingVisitor visitor) { - traceImpl(visitor); -} - -template <typename HolderType, typename ResolvedType, typename RejectedType> -template <typename VisitorDispatcher> -void ScriptPromiseProperty<HolderType, ResolvedType, RejectedType>::traceImpl( - VisitorDispatcher visitor) { TraceIfNeeded<HolderType>::trace(visitor, m_holder); TraceIfNeeded<ResolvedType>::trace(visitor, m_resolved); TraceIfNeeded<RejectedType>::trace(visitor, m_rejected);
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8HiddenValue.h b/third_party/WebKit/Source/bindings/core/v8/V8HiddenValue.h index fb82a0e..759b00a 100644 --- a/third_party/WebKit/Source/bindings/core/v8/V8HiddenValue.h +++ b/third_party/WebKit/Source/bindings/core/v8/V8HiddenValue.h
@@ -35,7 +35,6 @@ V(internalBodyStream) \ V(port1) \ V(port2) \ - V(readableStreamReaderInResponse) \ V(requestInFetchEvent) \ V(state) \ V(testInterfaces) \
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h b/third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h index d9cd41637..f81ba58 100644 --- a/third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h +++ b/third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h
@@ -21,19 +21,23 @@ class ScriptWrappable; // Apply |X| for each pair of (InterfaceName, PrivateKeyName). -#define V8_PRIVATE_PROPERTY_FOR_EACH(X) \ - X(CustomEvent, Detail) \ - X(DOMException, Error) \ - X(ErrorEvent, Error) \ - X(IDBObserver, Callback) \ - X(IntersectionObserver, Callback) \ - X(MessageEvent, CachedData) \ - X(MutationObserver, Callback) \ - X(PerformanceObserver, Callback) \ - X(SameObject, NotificationActions) \ - X(SameObject, NotificationData) \ - X(SameObject, NotificationVibrate) \ - X(V8NodeFilterCondition, Filter) \ +#define V8_PRIVATE_PROPERTY_FOR_EACH(X) \ + X(CustomEvent, Detail) \ + X(CustomElement, ConnectedCallback) \ + X(CustomElement, DisconnectedCallback) \ + X(CustomElement, AdoptedCallback) \ + X(CustomElement, AttributeChangedCallback) \ + X(DOMException, Error) \ + X(ErrorEvent, Error) \ + X(IDBObserver, Callback) \ + X(IntersectionObserver, Callback) \ + X(MessageEvent, CachedData) \ + X(MutationObserver, Callback) \ + X(PerformanceObserver, Callback) \ + X(SameObject, NotificationActions) \ + X(SameObject, NotificationData) \ + X(SameObject, NotificationVibrate) \ + X(V8NodeFilterCondition, Filter) \ X(Window, DocumentCachedAccessor) // The getter's name for a private property.
diff --git a/third_party/WebKit/Source/core/css/BUILD.gn b/third_party/WebKit/Source/core/css/BUILD.gn index 16517766f..bec5d64 100644 --- a/third_party/WebKit/Source/core/css/BUILD.gn +++ b/third_party/WebKit/Source/core/css/BUILD.gn
@@ -403,6 +403,7 @@ "properties/CSSPropertyAPIWebkitFontSizeDelta.cpp", "properties/CSSPropertyAPIWebkitHighlight.cpp", "properties/CSSPropertyAPIWebkitLineClamp.cpp", + "properties/CSSPropertyAPIWebkitMaxLogicalWidthOrHeight.cpp", "properties/CSSPropertyAPIWebkitPadding.cpp", "properties/CSSPropertyAPIWebkitTextEmphasisStyle.cpp", "properties/CSSPropertyAPIWebkitTextStrokeWidth.cpp",
diff --git a/third_party/WebKit/Source/core/css/CSSProperties.in b/third_party/WebKit/Source/core/css/CSSProperties.in index 1866879..c0a88360 100644 --- a/third_party/WebKit/Source/core/css/CSSProperties.in +++ b/third_party/WebKit/Source/core/css/CSSProperties.in
@@ -485,8 +485,8 @@ block-size direction_aware min-inline-size direction_aware min-block-size direction_aware -max-inline-size direction_aware -max-block-size direction_aware +max-inline-size direction_aware, api_class=CSSPropertyAPIWebkitMaxLogicalWidthOrHeight +max-block-size direction_aware, api_class=CSSPropertyAPIWebkitMaxLogicalWidthOrHeight // Non-standard direction aware properties @@ -514,8 +514,8 @@ -webkit-logical-height direction_aware -webkit-min-logical-width direction_aware -webkit-min-logical-height direction_aware --webkit-max-logical-width direction_aware --webkit-max-logical-height direction_aware +-webkit-max-logical-width direction_aware, api_class=CSSPropertyAPIWebkitMaxLogicalWidthOrHeight +-webkit-max-logical-height direction_aware, api_class=CSSPropertyAPIWebkitMaxLogicalWidthOrHeight // Properties that we ignore in the StyleBuilder. // TODO(timloh): This seems wrong, most of these shouldn't reach the StyleBuilder
diff --git a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp index a757e70d..87ef0bb 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp +++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -2106,12 +2106,6 @@ case CSSPropertyMaxHeight: return CSSPropertyLengthUtils::consumeMaxWidthOrHeight( m_range, m_context, UnitlessQuirk::Allow); - case CSSPropertyMaxInlineSize: - case CSSPropertyMaxBlockSize: - case CSSPropertyWebkitMaxLogicalWidth: - case CSSPropertyWebkitMaxLogicalHeight: - return CSSPropertyLengthUtils::consumeMaxWidthOrHeight(m_range, - m_context); case CSSPropertyMinWidth: case CSSPropertyMinHeight: case CSSPropertyWidth:
diff --git a/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIWebkitMaxLogicalWidthOrHeight.cpp b/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIWebkitMaxLogicalWidthOrHeight.cpp new file mode 100644 index 0000000..9013b29 --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIWebkitMaxLogicalWidthOrHeight.cpp
@@ -0,0 +1,17 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/css/properties/CSSPropertyAPIWebkitMaxLogicalWidthOrHeight.h" + +#include "core/css/properties/CSSPropertyLengthUtils.h" + +namespace blink { + +const CSSValue* CSSPropertyAPIWebkitMaxLogicalWidthOrHeight::parseSingleValue( + CSSParserTokenRange& range, + const CSSParserContext* context) { + return CSSPropertyLengthUtils::consumeMaxWidthOrHeight(range, context); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/dom/StaticNodeList.h b/third_party/WebKit/Source/core/dom/StaticNodeList.h index 9f6079e..7df9827 100644 --- a/third_party/WebKit/Source/core/dom/StaticNodeList.h +++ b/third_party/WebKit/Source/core/dom/StaticNodeList.h
@@ -81,17 +81,6 @@ template <typename NodeType> void StaticNodeTypeList<NodeType>::trace(Visitor* visitor) { - traceImpl(visitor); -} -template <typename NodeType> -void StaticNodeTypeList<NodeType>::trace(InlinedGlobalMarkingVisitor visitor) { - traceImpl(visitor); -} - -template <typename NodeType> -template <typename VisitorDispatcher> -ALWAYS_INLINE void StaticNodeTypeList<NodeType>::traceImpl( - VisitorDispatcher visitor) { visitor->trace(m_nodes); NodeList::trace(visitor); }
diff --git a/third_party/WebKit/Source/core/editing/VisibleUnits.cpp b/third_party/WebKit/Source/core/editing/VisibleUnits.cpp index 7366ca2..614ccd5 100644 --- a/third_party/WebKit/Source/core/editing/VisibleUnits.cpp +++ b/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
@@ -2703,9 +2703,10 @@ layoutTextFragment->firstLetterPseudoElement()->layoutObject(); // TODO(yosin): We're not sure when |firstLetterLayoutObject| has // multiple child layout object. - DCHECK_EQ(firstLetterLayoutObject->slowFirstChild(), - firstLetterLayoutObject->slowLastChild()); - return firstLetterLayoutObject->slowFirstChild(); + LayoutObject* child = firstLetterLayoutObject->slowFirstChild(); + CHECK(child && child->isText()); + DCHECK_EQ(child, firstLetterLayoutObject->slowLastChild()); + return child; } int caretMinOffset(const Node* node) {
diff --git a/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp b/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp index dfa646c..8c63902 100644 --- a/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp +++ b/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
@@ -835,12 +835,12 @@ return; LayoutObject* firstLetter = pseudoLayoutObject->slowFirstChild(); - DCHECK(firstLetter); - m_remainingTextBox = m_textBox; - m_textBox = toLayoutText(firstLetter)->firstTextBox(); m_sortedTextBoxes.clear(); + m_remainingTextBox = m_textBox; + CHECK(firstLetter && firstLetter->isText()); m_firstLetterText = toLayoutText(firstLetter); + m_textBox = m_firstLetterText->firstTextBox(); } template <typename Strategy>
diff --git a/third_party/WebKit/Source/core/loader/LinkLoaderClient.h b/third_party/WebKit/Source/core/loader/LinkLoaderClient.h index 59bff711..948b3ba7 100644 --- a/third_party/WebKit/Source/core/loader/LinkLoaderClient.h +++ b/third_party/WebKit/Source/core/loader/LinkLoaderClient.h
@@ -34,8 +34,7 @@ #include "core/CoreExport.h" #include "platform/WebTaskRunner.h" -#include "platform/heap/GarbageCollected.h" -#include "platform/heap/InlinedGlobalMarkingVisitor.h" +#include "platform/heap/Handle.h" namespace blink {
diff --git a/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp b/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp index 4435f0b..5e72a29 100644 --- a/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp +++ b/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
@@ -952,12 +952,6 @@ if (!m_sendFlag || m_loader) return; } - if (!getExecutionContext()) { - handleNetworkError(); - throwForLoadFailureIfNeeded(exceptionState, - "Document is already detached."); - return; - } } m_sameOriginRequest = getSecurityOrigin()->canRequestNoSuborigin(m_url); @@ -1818,6 +1812,10 @@ m_method, m_url); m_progressEventThrottle->stop(); internalAbort(); + + // In case we are in the middle of send() function, unset the send flag to + // stop the operation. + m_sendFlag = false; } bool XMLHttpRequest::hasPendingActivity() const {
diff --git a/third_party/WebKit/Source/devtools/front_end/ui/Icon.js b/third_party/WebKit/Source/devtools/front_end/ui/Icon.js index 49d041b..ed9fda13 100644 --- a/third_party/WebKit/Source/devtools/front_end/ui/Icon.js +++ b/third_party/WebKit/Source/devtools/front_end/ui/Icon.js
@@ -170,8 +170,10 @@ 'largeicon-show-left-sidebar': {x: -160, y: -72, width: 28, height: 24, spritesheet: 'largeicons', isMask: true}, 'largeicon-hide-left-sidebar': {x: -192, y: -72, width: 28, height: 24, spritesheet: 'largeicons', isMask: true}, - 'largeicon-show-right-sidebar': {x: -192, y: -72, width: 28, height: 24, spritesheet: 'largeicons', isMask: true}, - 'largeicon-hide-right-sidebar': {x: -160, y: -72, width: 28, height: 24, spritesheet: 'largeicons', isMask: true}, + 'largeicon-show-right-sidebar': + {x: -160, y: -72, width: 28, height: 24, spritesheet: 'largeicons', isMask: true, transform: 'rotate(180deg)'}, + 'largeicon-hide-right-sidebar': + {x: -192, y: -72, width: 28, height: 24, spritesheet: 'largeicons', isMask: true, transform: 'rotate(180deg)'}, 'largeicon-show-top-sidebar': { x: -160, y: -72,
diff --git a/third_party/WebKit/Source/modules/fetch/Response.cpp b/third_party/WebKit/Source/modules/fetch/Response.cpp index 71e3ac0..63bc3bd 100644 --- a/third_party/WebKit/Source/modules/fetch/Response.cpp +++ b/third_party/WebKit/Source/modules/fetch/Response.cpp
@@ -130,7 +130,6 @@ const Dictionary& init, ExceptionState& exceptionState) { v8::Local<v8::Value> body = bodyValue.v8Value(); - ScriptValue reader; v8::Isolate* isolate = scriptState->isolate(); ExecutionContext* executionContext = scriptState->getExecutionContext(); @@ -181,25 +180,8 @@ new BodyStreamBuffer(scriptState, new FormDataBytesConsumer(string)); contentType = "text/plain;charset=UTF-8"; } - Response* response = - create(scriptState, bodyBuffer, contentType, - ResponseInit(init, exceptionState), exceptionState); - if (!exceptionState.hadException() && !reader.isEmpty()) { - // Add a hidden reference so that the weak persistent in the - // ReadableStreamBytesConsumer will be valid as long as the - // Response is valid. - v8::Local<v8::Value> wrapper = ToV8(response, scriptState); - if (wrapper.IsEmpty()) { - exceptionState.throwTypeError("Cannot create a Response wrapper"); - return nullptr; - } - ASSERT(wrapper->IsObject()); - V8HiddenValue::setHiddenValue( - scriptState, wrapper.As<v8::Object>(), - V8HiddenValue::readableStreamReaderInResponse(scriptState->isolate()), - reader.v8Value()); - } - return response; + return create(scriptState, bodyBuffer, contentType, + ResponseInit(init, exceptionState), exceptionState); } Response* Response::create(ScriptState* scriptState,
diff --git a/third_party/WebKit/Source/platform/heap/BUILD.gn b/third_party/WebKit/Source/platform/heap/BUILD.gn index 4fc88f7f..ff9abe99 100644 --- a/third_party/WebKit/Source/platform/heap/BUILD.gn +++ b/third_party/WebKit/Source/platform/heap/BUILD.gn
@@ -30,9 +30,6 @@ "HeapCompact.h", "HeapPage.cpp", "HeapPage.h", - "InlinedGlobalMarkingVisitor.h", - "MarkingVisitor.h", - "MarkingVisitorImpl.h", "Member.h", "PageMemory.cpp", "PageMemory.h", @@ -54,6 +51,7 @@ "TraceTraits.h", "Visitor.cpp", "Visitor.h", + "VisitorImpl.h", "WrapperVisitor.h", ]
diff --git a/third_party/WebKit/Source/platform/heap/GarbageCollected.h b/third_party/WebKit/Source/platform/heap/GarbageCollected.h index 79a37b3..eb6c316 100644 --- a/third_party/WebKit/Source/platform/heap/GarbageCollected.h +++ b/third_party/WebKit/Source/platform/heap/GarbageCollected.h
@@ -14,7 +14,6 @@ template <typename T> class GarbageCollected; -class InlinedGlobalMarkingVisitor; class TraceWrapperBase; // GC_PLUGIN_IGNORE is used to make the plugin ignore a particular class or @@ -73,8 +72,6 @@ typedef int IsGarbageCollectedMixinMarker; virtual void adjustAndMark(Visitor*) const = 0; virtual void trace(Visitor*) {} - virtual void adjustAndMark(InlinedGlobalMarkingVisitor) const = 0; - virtual void trace(InlinedGlobalMarkingVisitor); virtual bool isHeapObjectAlive() const = 0; }; @@ -155,8 +152,6 @@ #define USING_GARBAGE_COLLECTED_MIXIN(TYPE) \ IS_GARBAGE_COLLECTED_TYPE(); \ DEFINE_GARBAGE_COLLECTED_MIXIN_METHODS(blink::Visitor*, TYPE) \ - DEFINE_GARBAGE_COLLECTED_MIXIN_METHODS(blink::InlinedGlobalMarkingVisitor, \ - TYPE) \ DEFINE_GARBAGE_COLLECTED_MIXIN_CONSTRUCTOR_MARKER(TYPE) \ public: \ bool isHeapObjectAlive() const override { \
diff --git a/third_party/WebKit/Source/platform/heap/Handle.h b/third_party/WebKit/Source/platform/heap/Handle.h index 882d6e93..ef31d2d 100644 --- a/third_party/WebKit/Source/platform/heap/Handle.h +++ b/third_party/WebKit/Source/platform/heap/Handle.h
@@ -33,12 +33,12 @@ #include "platform/heap/Heap.h" #include "platform/heap/HeapAllocator.h" -#include "platform/heap/InlinedGlobalMarkingVisitor.h" #include "platform/heap/Member.h" #include "platform/heap/Persistent.h" #include "platform/heap/ThreadState.h" #include "platform/heap/TraceTraits.h" #include "platform/heap/Visitor.h" +#include "platform/heap/VisitorImpl.h" #include "wtf/Allocator.h" #if defined(LEAK_SANITIZER)
diff --git a/third_party/WebKit/Source/platform/heap/Heap.cpp b/third_party/WebKit/Source/platform/heap/Heap.cpp index 22e5ef818..7dd5529 100644 --- a/third_party/WebKit/Source/platform/heap/Heap.cpp +++ b/third_party/WebKit/Source/platform/heap/Heap.cpp
@@ -36,7 +36,6 @@ #include "platform/heap/BlinkGCMemoryDumpProvider.h" #include "platform/heap/CallbackStack.h" #include "platform/heap/HeapCompact.h" -#include "platform/heap/MarkingVisitor.h" #include "platform/heap/PageMemory.h" #include "platform/heap/PagePool.h" #include "platform/heap/SafePoint.h" @@ -316,6 +315,30 @@ return nullptr; } +#if DCHECK_IS_ON() +// To support unit testing of the marking of off-heap root references +// into the heap, provide a checkAndMarkPointer() version with an +// extra notification argument. +Address ThreadHeap::checkAndMarkPointer( + Visitor* visitor, + Address address, + MarkedPointerCallbackForTesting callback) { + DCHECK(ThreadState::current()->isInGC()); + + if (BasePage* page = lookupPageForAddress(address)) { + DCHECK(page->contains(address)); + DCHECK(!page->orphaned()); + DCHECK(!m_heapDoesNotContainCache->lookup(address)); + DCHECK(&visitor->heap() == &page->arena()->getThreadState()->heap()); + page->checkAndMarkPointer(visitor, address, callback); + return address; + } + if (!m_heapDoesNotContainCache->lookup(address)) + m_heapDoesNotContainCache->addEntry(address); + return nullptr; +} +#endif + void ThreadHeap::pushTraceCallback(void* object, TraceCallback callback) { ASSERT(ThreadState::current()->isInGC());
diff --git a/third_party/WebKit/Source/platform/heap/Heap.h b/third_party/WebKit/Source/platform/heap/Heap.h index c494896d1..fd9efc0 100644 --- a/third_party/WebKit/Source/platform/heap/Heap.h +++ b/third_party/WebKit/Source/platform/heap/Heap.h
@@ -435,6 +435,11 @@ // Conservatively checks whether an address is a pointer in any of the // thread heaps. If so marks the object pointed to as live. Address checkAndMarkPointer(Visitor*, Address); +#if DCHECK_IS_ON() + Address checkAndMarkPointer(Visitor*, + Address, + MarkedPointerCallbackForTesting); +#endif size_t objectPayloadSizeForTesting(); @@ -692,9 +697,8 @@ return address; } -template <typename Derived> template <typename T> -void VisitorHelper<Derived>::handleWeakCell(Visitor* self, void* object) { +void Visitor::handleWeakCell(Visitor* self, void* object) { T** cell = reinterpret_cast<T**>(object); if (*cell && !ObjectAliveTrait<T>::isHeapObjectAlive(*cell)) *cell = nullptr; @@ -702,4 +706,6 @@ } // namespace blink +#include "platform/heap/VisitorImpl.h" + #endif // Heap_h
diff --git a/third_party/WebKit/Source/platform/heap/HeapPage.cpp b/third_party/WebKit/Source/platform/heap/HeapPage.cpp index cb8f77ef..79c74b5 100644 --- a/third_party/WebKit/Source/platform/heap/HeapPage.cpp +++ b/third_party/WebKit/Source/platform/heap/HeapPage.cpp
@@ -36,7 +36,6 @@ #include "platform/heap/BlinkGCMemoryDumpProvider.h" #include "platform/heap/CallbackStack.h" #include "platform/heap/HeapCompact.h" -#include "platform/heap/MarkingVisitor.h" #include "platform/heap/PageMemory.h" #include "platform/heap/PagePool.h" #include "platform/heap/SafePoint.h" @@ -1700,13 +1699,28 @@ } void NormalPage::checkAndMarkPointer(Visitor* visitor, Address address) { - ASSERT(contains(address)); +#if DCHECK_IS_ON() + DCHECK(contains(address)); +#endif HeapObjectHeader* header = findHeaderFromAddress(address); if (!header || header->isDead()) return; markPointer(visitor, header); } +#if DCHECK_IS_ON() +void NormalPage::checkAndMarkPointer(Visitor* visitor, + Address address, + MarkedPointerCallbackForTesting callback) { + DCHECK(contains(address)); + HeapObjectHeader* header = findHeaderFromAddress(address); + if (!header || header->isDead()) + return; + if (!callback(header)) + markPointer(visitor, header); +} +#endif + void NormalPage::markOrphaned() { // Zap the payload with a recognizable value to detect any incorrect // cross thread pointer usage. @@ -1827,12 +1841,27 @@ #endif void LargeObjectPage::checkAndMarkPointer(Visitor* visitor, Address address) { - ASSERT(contains(address)); +#if DCHECK_IS_ON() + DCHECK(contains(address)); +#endif if (!containedInObjectPayload(address) || heapObjectHeader()->isDead()) return; markPointer(visitor, heapObjectHeader()); } +#if DCHECK_IS_ON() +void LargeObjectPage::checkAndMarkPointer( + Visitor* visitor, + Address address, + MarkedPointerCallbackForTesting callback) { + DCHECK(contains(address)); + if (!containedInObjectPayload(address) || heapObjectHeader()->isDead()) + return; + if (!callback(heapObjectHeader())) + markPointer(visitor, heapObjectHeader()); +} +#endif + void LargeObjectPage::markOrphaned() { // Zap the payload with a recognizable value to detect any incorrect // cross thread pointer usage.
diff --git a/third_party/WebKit/Source/platform/heap/HeapPage.h b/third_party/WebKit/Source/platform/heap/HeapPage.h index c401c9ef..233d8de 100644 --- a/third_party/WebKit/Source/platform/heap/HeapPage.h +++ b/third_party/WebKit/Source/platform/heap/HeapPage.h
@@ -354,6 +354,13 @@ return !((reinterpret_cast<uintptr_t>(address) & blinkPageOffsetMask) - blinkGuardPageSize); } + +// Callback used for unit testing the marking of conservative pointers +// (checkAndMarkPointer().) For each pointer that has been discovered +// to point to a heap object, the callback is invoked with a pointer +// to its header. If the callback returns |true|, the object will not +// be marked. +using MarkedPointerCallbackForTesting = bool (*)(HeapObjectHeader*); #endif // BasePage is a base class for NormalPage and LargeObjectPage. @@ -408,6 +415,11 @@ // conservatively mark all objects that could be referenced from // the stack. virtual void checkAndMarkPointer(Visitor*, Address) = 0; +#if DCHECK_IS_ON() + virtual void checkAndMarkPointer(Visitor*, + Address, + MarkedPointerCallbackForTesting) = 0; +#endif virtual void markOrphaned(); class HeapSnapshotInfo { @@ -489,6 +501,11 @@ void poisonUnmarkedObjects() override; #endif void checkAndMarkPointer(Visitor*, Address) override; +#if DCHECK_IS_ON() + void checkAndMarkPointer(Visitor*, + Address, + MarkedPointerCallbackForTesting) override; +#endif void markOrphaned() override; void takeSnapshot(base::trace_event::MemoryAllocatorDump*, @@ -567,6 +584,11 @@ void poisonUnmarkedObjects() override; #endif void checkAndMarkPointer(Visitor*, Address) override; +#if DCHECK_IS_ON() + void checkAndMarkPointer(Visitor*, + Address, + MarkedPointerCallbackForTesting) override; +#endif void markOrphaned() override; void takeSnapshot(base::trace_event::MemoryAllocatorDump*,
diff --git a/third_party/WebKit/Source/platform/heap/HeapTest.cpp b/third_party/WebKit/Source/platform/heap/HeapTest.cpp index 9969e20d..3a85dc2 100644 --- a/third_party/WebKit/Source/platform/heap/HeapTest.cpp +++ b/third_party/WebKit/Source/platform/heap/HeapTest.cpp
@@ -305,60 +305,6 @@ bool m_parkedAllThreads; // False if we fail to park all threads }; -#define DEFINE_VISITOR_METHODS(Type) \ - void mark(const Type* object, TraceCallback callback) override { \ - if (object) \ - m_count++; \ - } \ - bool isMarked(const Type*) override { return false; } \ - bool ensureMarked(const Type* objectPointer) override { \ - return ensureMarked(objectPointer); \ - } - -class CountingVisitor : public Visitor { - public: - explicit CountingVisitor(ThreadState* state) - : Visitor(state, VisitorMarkingMode::ThreadLocalMarking), - m_scope(&state->heap().stackFrameDepth()), - m_count(0) {} - - void mark(const void* object, TraceCallback) override { - if (object) - m_count++; - } - - void markHeader(HeapObjectHeader* header, TraceCallback callback) override { - ASSERT(header->payload()); - m_count++; - } - - void registerDelayedMarkNoTracing(const void*) override {} - void registerWeakMembers(const void*, const void*, WeakCallback) override {} - void registerWeakTable(const void*, - EphemeronCallback, - EphemeronCallback) override {} -#if DCHECK_IS_ON() - bool weakTableRegistered(const void*) override { return false; } -#endif - void registerWeakCellWithCallback(void**, WeakCallback) override {} - bool ensureMarked(const void* objectPointer) override { - if (!objectPointer || - HeapObjectHeader::fromPayload(objectPointer)->isMarked()) - return false; - markNoTracing(objectPointer); - return true; - } - - size_t count() { return m_count; } - void reset() { m_count = 0; } - - private: - StackFrameDepthScope m_scope; - size_t m_count; -}; - -#undef DEFINE_VISITOR_METHODS - class SimpleObject : public GarbageCollected<SimpleObject> { public: static SimpleObject* create() { return new SimpleObject(); } @@ -1020,58 +966,6 @@ int RefCountedAndGarbageCollected2::s_destructorCalls = 0; -#define DEFINE_VISITOR_METHODS(Type) \ - void mark(const Type* object, TraceCallback callback) override { \ - mark(object); \ - } - -class RefCountedGarbageCollectedVisitor : public CountingVisitor { - public: - RefCountedGarbageCollectedVisitor(ThreadState* state, - int expected, - void** objects) - : CountingVisitor(state), - m_count(0), - m_expectedCount(expected), - m_expectedObjects(objects) {} - - void mark(const void* ptr) { markNoTrace(ptr); } - - virtual void markNoTrace(const void* ptr) { - if (!ptr) - return; - if (m_count < m_expectedCount) - EXPECT_TRUE(expectedObject(ptr)); - else - EXPECT_FALSE(expectedObject(ptr)); - m_count++; - } - - void mark(const void* ptr, TraceCallback) override { mark(ptr); } - - void markHeader(HeapObjectHeader* header, TraceCallback callback) override { - mark(header->payload()); - } - - bool validate() { return m_count >= m_expectedCount; } - void reset() { m_count = 0; } - - private: - bool expectedObject(const void* ptr) { - for (int i = 0; i < m_expectedCount; i++) { - if (m_expectedObjects[i] == ptr) - return true; - } - return false; - } - - int m_count; - int m_expectedCount; - void** m_expectedObjects; -}; - -#undef DEFINE_VISITOR_METHODS - class Weak : public Bar { public: static Weak* create(Bar* strong, Bar* weak) { return new Weak(strong, weak); } @@ -3745,69 +3639,6 @@ EXPECT_EQ(2, RefCountedAndGarbageCollected::s_destructorCalls); } -TEST(HeapTest, RefCountedGarbageCollectedWithStackPointers) { - RefCountedAndGarbageCollected::s_destructorCalls = 0; - RefCountedAndGarbageCollected2::s_destructorCalls = 0; - { - RefCountedAndGarbageCollected* pointer1 = 0; - RefCountedAndGarbageCollected2* pointer2 = 0; - { - Persistent<RefCountedAndGarbageCollected> object1 = - RefCountedAndGarbageCollected::create(); - Persistent<RefCountedAndGarbageCollected2> object2 = - RefCountedAndGarbageCollected2::create(); - pointer1 = object1.get(); - pointer2 = object2.get(); - void* objects[2] = {object1.get(), object2.get()}; - ThreadState::GCForbiddenScope gcScope(ThreadState::current()); - RefCountedGarbageCollectedVisitor visitor(ThreadState::current(), 2, - objects); - ThreadState::current()->visitPersistents(&visitor); - EXPECT_TRUE(visitor.validate()); - } - conservativelyCollectGarbage(); - EXPECT_EQ(0, RefCountedAndGarbageCollected::s_destructorCalls); - EXPECT_EQ(0, RefCountedAndGarbageCollected2::s_destructorCalls); - - conservativelyCollectGarbage(); - EXPECT_EQ(0, RefCountedAndGarbageCollected::s_destructorCalls); - EXPECT_EQ(0, RefCountedAndGarbageCollected2::s_destructorCalls); - - { - // At this point, the reference counts of object1 and object2 are 0. - // Only pointer1 and pointer2 keep references to object1 and object2. - void* objects[] = {0}; - ThreadState::GCForbiddenScope gcScope(ThreadState::current()); - RefCountedGarbageCollectedVisitor visitor(ThreadState::current(), 0, - objects); - ThreadState::current()->visitPersistents(&visitor); - EXPECT_TRUE(visitor.validate()); - } - - { - Persistent<RefCountedAndGarbageCollected> object1(pointer1); - Persistent<RefCountedAndGarbageCollected2> object2(pointer2); - void* objects[2] = {object1.get(), object2.get()}; - ThreadState::GCForbiddenScope gcScope(ThreadState::current()); - RefCountedGarbageCollectedVisitor visitor(ThreadState::current(), 2, - objects); - ThreadState::current()->visitPersistents(&visitor); - EXPECT_TRUE(visitor.validate()); - } - conservativelyCollectGarbage(); - EXPECT_EQ(0, RefCountedAndGarbageCollected::s_destructorCalls); - EXPECT_EQ(0, RefCountedAndGarbageCollected2::s_destructorCalls); - - conservativelyCollectGarbage(); - EXPECT_EQ(0, RefCountedAndGarbageCollected::s_destructorCalls); - EXPECT_EQ(0, RefCountedAndGarbageCollected2::s_destructorCalls); - } - - preciselyCollectGarbage(); - EXPECT_EQ(1, RefCountedAndGarbageCollected::s_destructorCalls); - EXPECT_EQ(1, RefCountedAndGarbageCollected2::s_destructorCalls); -} - TEST(HeapTest, WeakMembers) { Bar::s_live = 0; { @@ -3911,7 +3742,21 @@ EXPECT_TRUE(barPersistent == fooPersistent); } +#if DCHECK_IS_ON() +namespace { + +static size_t s_checkMarkCount = 0; + +bool reportMarkedPointer(HeapObjectHeader*) { + s_checkMarkCount++; + // Do not try to mark the located heap object. + return true; +} +} +#endif + TEST(HeapTest, CheckAndMarkPointer) { +#if DCHECK_IS_ON() ThreadHeap& heap = ThreadState::current()->heap(); clearOutOldGarbage(); @@ -3939,20 +3784,25 @@ { TestGCScope scope(BlinkGC::HeapPointersOnStack); ThreadState::GCForbiddenScope gcScope(ThreadState::current()); - CountingVisitor visitor(ThreadState::current()); + Visitor visitor(ThreadState::current(), + VisitorMarkingMode::ThreadLocalMarking); EXPECT_TRUE(scope.allThreadsParked()); // Fail the test if we could not // park all threads. heap.flushHeapDoesNotContainCache(); for (size_t i = 0; i < objectAddresses.size(); i++) { - EXPECT_TRUE(heap.checkAndMarkPointer(&visitor, objectAddresses[i])); - EXPECT_TRUE(heap.checkAndMarkPointer(&visitor, endAddresses[i])); + EXPECT_TRUE(heap.checkAndMarkPointer(&visitor, objectAddresses[i], + reportMarkedPointer)); + EXPECT_TRUE(heap.checkAndMarkPointer(&visitor, endAddresses[i], + reportMarkedPointer)); } - EXPECT_EQ(objectAddresses.size() * 2, visitor.count()); - visitor.reset(); - EXPECT_TRUE(heap.checkAndMarkPointer(&visitor, largeObjectAddress)); - EXPECT_TRUE(heap.checkAndMarkPointer(&visitor, largeObjectEndAddress)); - EXPECT_EQ(2ul, visitor.count()); - visitor.reset(); + EXPECT_EQ(objectAddresses.size() * 2, s_checkMarkCount); + s_checkMarkCount = 0; + EXPECT_TRUE(heap.checkAndMarkPointer(&visitor, largeObjectAddress, + reportMarkedPointer)); + EXPECT_TRUE(heap.checkAndMarkPointer(&visitor, largeObjectEndAddress, + reportMarkedPointer)); + EXPECT_EQ(2ul, s_checkMarkCount); + s_checkMarkCount = 0ul; } // This forces a GC without stack scanning which results in the objects // being collected. This will also rebuild the above mentioned freelists, @@ -3961,7 +3811,8 @@ { TestGCScope scope(BlinkGC::HeapPointersOnStack); ThreadState::GCForbiddenScope gcScope(ThreadState::current()); - CountingVisitor visitor(ThreadState::current()); + Visitor visitor(ThreadState::current(), + VisitorMarkingMode::ThreadLocalMarking); EXPECT_TRUE(scope.allThreadsParked()); heap.flushHeapDoesNotContainCache(); for (size_t i = 0; i < objectAddresses.size(); i++) { @@ -3972,17 +3823,20 @@ // whether it points at a valid object (this ensures the // correctness of the page-based on-heap address caches), so we // can't make that assert. - heap.checkAndMarkPointer(&visitor, objectAddresses[i]); - heap.checkAndMarkPointer(&visitor, endAddresses[i]); + heap.checkAndMarkPointer(&visitor, objectAddresses[i], + reportMarkedPointer); + heap.checkAndMarkPointer(&visitor, endAddresses[i], reportMarkedPointer); } - EXPECT_EQ(0ul, visitor.count()); - heap.checkAndMarkPointer(&visitor, largeObjectAddress); - heap.checkAndMarkPointer(&visitor, largeObjectEndAddress); - EXPECT_EQ(0ul, visitor.count()); + EXPECT_EQ(0ul, s_checkMarkCount); + heap.checkAndMarkPointer(&visitor, largeObjectAddress, reportMarkedPointer); + heap.checkAndMarkPointer(&visitor, largeObjectEndAddress, + reportMarkedPointer); + EXPECT_EQ(0ul, s_checkMarkCount); } // This round of GC is important to make sure that the object start // bitmap are cleared out and that the free lists are rebuild. clearOutOldGarbage(); +#endif } TEST(HeapTest, PersistentHeapCollectionTypes) { @@ -5626,45 +5480,6 @@ Member<SimpleObject> m_obj; }; -TEST(HeapTest, TraceIfNeeded) { - ThreadState::GCForbiddenScope scope(ThreadState::current()); - CountingVisitor visitor(ThreadState::current()); - - { - TraceIfNeededTester<RefPtr<OffHeapInt>>* m_offHeap = - TraceIfNeededTester<RefPtr<OffHeapInt>>::create(OffHeapInt::create(42)); - visitor.reset(); - m_offHeap->trace(&visitor); - EXPECT_EQ(0u, visitor.count()); - } - - { - TraceIfNeededTester<PartObject>* m_part = - TraceIfNeededTester<PartObject>::create(); - visitor.reset(); - m_part->trace(&visitor); - EXPECT_EQ(1u, visitor.count()); - } - - { - TraceIfNeededTester<Member<SimpleObject>>* m_obj = - TraceIfNeededTester<Member<SimpleObject>>::create( - Member<SimpleObject>(SimpleObject::create())); - visitor.reset(); - m_obj->trace(&visitor); - EXPECT_EQ(1u, visitor.count()); - } - - { - TraceIfNeededTester<HeapVector<Member<SimpleObject>>>* m_vec = - TraceIfNeededTester<HeapVector<Member<SimpleObject>>>::create(); - m_vec->obj().push_back(SimpleObject::create()); - visitor.reset(); - m_vec->trace(&visitor); - EXPECT_EQ(2u, visitor.count()); - } -} - class AllocatesOnAssignment { public: AllocatesOnAssignment(std::nullptr_t) : m_value(nullptr) {}
diff --git a/third_party/WebKit/Source/platform/heap/InlinedGlobalMarkingVisitor.h b/third_party/WebKit/Source/platform/heap/InlinedGlobalMarkingVisitor.h deleted file mode 100644 index ec8d5dbc..0000000 --- a/third_party/WebKit/Source/platform/heap/InlinedGlobalMarkingVisitor.h +++ /dev/null
@@ -1,60 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef InlinedGlobalMarkingVisitor_h -#define InlinedGlobalMarkingVisitor_h - -#include "platform/heap/MarkingVisitorImpl.h" - -namespace blink { - -class InlinedGlobalMarkingVisitor final - : public VisitorHelper<InlinedGlobalMarkingVisitor>, - public MarkingVisitorImpl<InlinedGlobalMarkingVisitor> { - public: - friend class VisitorHelper<InlinedGlobalMarkingVisitor>; - using Helper = VisitorHelper<InlinedGlobalMarkingVisitor>; - friend class MarkingVisitorImpl<InlinedGlobalMarkingVisitor>; - using Impl = MarkingVisitorImpl<InlinedGlobalMarkingVisitor>; - - InlinedGlobalMarkingVisitor(ThreadState* state, - VisitorMarkingMode markingMode) - : VisitorHelper(state, markingMode) {} - - // Hack to unify interface to visitor->trace(). - // Without this hack, we need to use visitor.trace() for - // trace(InlinedGlobalMarkingVisitor) and visitor->trace() for - // trace(Visitor*). - InlinedGlobalMarkingVisitor* operator->() { return this; } - - using Impl::mark; - using Impl::ensureMarked; - using Impl::registerDelayedMarkNoTracing; - using Impl::registerWeakTable; - using Impl::registerWeakMembers; -#if DCHECK_IS_ON() - using Impl::weakTableRegistered; -#endif - - template <typename T> - void mark(T* t) { - Helper::mark(t); - } - - template <typename T, void (T::*method)(Visitor*)> - void registerWeakMembers(const T* obj) { - Helper::template registerWeakMembers<T, method>(obj); - } - - private: - static InlinedGlobalMarkingVisitor fromHelper(Helper* helper) { - return *static_cast<InlinedGlobalMarkingVisitor*>(helper); - } -}; - -inline void GarbageCollectedMixin::trace(InlinedGlobalMarkingVisitor) {} - -} // namespace blink - -#endif
diff --git a/third_party/WebKit/Source/platform/heap/MarkingVisitor.h b/third_party/WebKit/Source/platform/heap/MarkingVisitor.h deleted file mode 100644 index 9a07e69..0000000 --- a/third_party/WebKit/Source/platform/heap/MarkingVisitor.h +++ /dev/null
@@ -1,62 +0,0 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef MarkingVisitor_h -#define MarkingVisitor_h - -#include "platform/heap/MarkingVisitorImpl.h" - -namespace blink { - -class MarkingVisitor final : public Visitor, - public MarkingVisitorImpl<MarkingVisitor> { - public: - using Impl = MarkingVisitorImpl<MarkingVisitor>; - - MarkingVisitor(ThreadState* state, VisitorMarkingMode mode) - : Visitor(state, mode) {} - - void markHeader(HeapObjectHeader* header, TraceCallback callback) override { - Impl::markHeader(header, header->payload(), callback); - } - - void mark(const void* objectPointer, TraceCallback callback) override { - Impl::mark(objectPointer, callback); - } - - void registerDelayedMarkNoTracing(const void* object) override { - Impl::registerDelayedMarkNoTracing(object); - } - - void registerWeakMembers(const void* closure, - const void* objectPointer, - WeakCallback callback) override { - Impl::registerWeakMembers(closure, objectPointer, callback); - } - - virtual void registerWeakTable(const void* closure, - EphemeronCallback iterationCallback, - EphemeronCallback iterationDoneCallback) { - Impl::registerWeakTable(closure, iterationCallback, iterationDoneCallback); - } - -#if DCHECK_IS_ON() - virtual bool weakTableRegistered(const void* closure) { - return Impl::weakTableRegistered(closure); - } -#endif - - bool ensureMarked(const void* objectPointer) override { - return Impl::ensureMarked(objectPointer); - } - - void registerWeakCellWithCallback(void** cell, - WeakCallback callback) override { - Impl::registerWeakCellWithCallback(cell, callback); - } -}; - -} // namespace blink - -#endif
diff --git a/third_party/WebKit/Source/platform/heap/MarkingVisitorImpl.h b/third_party/WebKit/Source/platform/heap/MarkingVisitorImpl.h deleted file mode 100644 index 5394e53..0000000 --- a/third_party/WebKit/Source/platform/heap/MarkingVisitorImpl.h +++ /dev/null
@@ -1,124 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef MarkingVisitorImpl_h -#define MarkingVisitorImpl_h - -#include "platform/heap/Heap.h" -#include "platform/heap/ThreadState.h" -#include "platform/heap/Visitor.h" -#include "wtf/Allocator.h" - -namespace blink { - -template <typename Derived> -class MarkingVisitorImpl { - USING_FAST_MALLOC(MarkingVisitorImpl); - - protected: - inline void markHeader(HeapObjectHeader* header, - const void* objectPointer, - TraceCallback callback) { - ASSERT(header); - ASSERT(objectPointer); - - // If you hit this ASSERT, it means that there is a dangling pointer - // from a live thread heap to a dead thread heap. We must eliminate - // the dangling pointer. - // Release builds don't have the ASSERT, but it is OK because - // release builds will crash in the following header->isMarked() - // because all the entries of the orphaned arenas are zapped. - ASSERT(!pageFromObject(objectPointer)->orphaned()); - - if (header->isMarked()) - return; - - ASSERT(ThreadState::current()->isInGC()); - DCHECK(toDerived()->getMarkingMode() != VisitorMarkingMode::WeakProcessing); - - // A GC should only mark the objects that belong in its heap. - DCHECK(&pageFromObject(objectPointer)->arena()->getThreadState()->heap() == - &toDerived()->heap()); - - header->mark(); - - if (callback) - toDerived()->heap().pushTraceCallback(const_cast<void*>(objectPointer), - callback); - } - - inline void mark(const void* objectPointer, TraceCallback callback) { - if (!objectPointer) - return; - HeapObjectHeader* header = HeapObjectHeader::fromPayload(objectPointer); - markHeader(header, header->payload(), callback); - } - - inline void registerDelayedMarkNoTracing(const void* objectPointer) { - DCHECK(toDerived()->getMarkingMode() != VisitorMarkingMode::WeakProcessing); - toDerived()->heap().pushPostMarkingCallback( - const_cast<void*>(objectPointer), &markNoTracingCallback); - } - - inline void registerWeakMembers(const void* closure, - const void* objectPointer, - WeakCallback callback) { - DCHECK(toDerived()->getMarkingMode() != VisitorMarkingMode::WeakProcessing); - // We don't want to run weak processings when taking a snapshot. - if (toDerived()->getMarkingMode() == VisitorMarkingMode::SnapshotMarking) - return; - toDerived()->heap().pushThreadLocalWeakCallback( - const_cast<void*>(closure), const_cast<void*>(objectPointer), callback); - } - - inline void registerWeakTable(const void* closure, - EphemeronCallback iterationCallback, - EphemeronCallback iterationDoneCallback) { - DCHECK(toDerived()->getMarkingMode() != VisitorMarkingMode::WeakProcessing); - toDerived()->heap().registerWeakTable( - const_cast<void*>(closure), iterationCallback, iterationDoneCallback); - } - -#if DCHECK_IS_ON() - inline bool weakTableRegistered(const void* closure) { - return toDerived()->heap().weakTableRegistered(closure); - } -#endif - - inline bool ensureMarked(const void* objectPointer) { - if (!objectPointer) - return false; - - HeapObjectHeader* header = HeapObjectHeader::fromPayload(objectPointer); - if (header->isMarked()) - return false; -#if DCHECK_IS_ON() - toDerived()->markNoTracing(objectPointer); -#else - // Inline what the above markNoTracing() call expands to, - // so as to make sure that we do get all the benefits (asserts excepted.) - header->mark(); -#endif - return true; - } - - inline void registerWeakCellWithCallback(void** cell, WeakCallback callback) { - DCHECK(toDerived()->getMarkingMode() != VisitorMarkingMode::WeakProcessing); - // We don't want to run weak processings when taking a snapshot. - if (toDerived()->getMarkingMode() == VisitorMarkingMode::SnapshotMarking) - return; - toDerived()->heap().pushGlobalWeakCallback(cell, callback); - } - - Derived* toDerived() { return static_cast<Derived*>(this); } - - private: - static void markNoTracingCallback(Visitor* visitor, void* object) { - visitor->markNoTracing(object); - } -}; - -} // namespace blink - -#endif
diff --git a/third_party/WebKit/Source/platform/heap/Member.h b/third_party/WebKit/Source/platform/heap/Member.h index db16863..b2bac89 100644 --- a/third_party/WebKit/Source/platform/heap/Member.h +++ b/third_party/WebKit/Source/platform/heap/Member.h
@@ -434,8 +434,7 @@ private: T** cell() const { return const_cast<T**>(&this->m_raw); } - template <typename Derived> - friend class VisitorHelper; + friend class Visitor; }; // UntracedMember is a pointer to an on-heap object that is not traced for some
diff --git a/third_party/WebKit/Source/platform/heap/RunAllTests.cpp b/third_party/WebKit/Source/platform/heap/RunAllTests.cpp index bd6c94d..1f3e9d65 100644 --- a/third_party/WebKit/Source/platform/heap/RunAllTests.cpp +++ b/third_party/WebKit/Source/platform/heap/RunAllTests.cpp
@@ -28,7 +28,7 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "platform/heap/Heap.h" +#include "platform/heap/Handle.h" #include <base/bind.h> #include <base/test/launcher/unit_test_launcher.h> #include <base/test/test_suite.h>
diff --git a/third_party/WebKit/Source/platform/heap/TraceTraits.h b/third_party/WebKit/Source/platform/heap/TraceTraits.h index 7e967449e..fb5b1a0 100644 --- a/third_party/WebKit/Source/platform/heap/TraceTraits.h +++ b/third_party/WebKit/Source/platform/heap/TraceTraits.h
@@ -7,7 +7,6 @@ #include "platform/heap/GCInfo.h" #include "platform/heap/Heap.h" -#include "platform/heap/InlinedGlobalMarkingVisitor.h" #include "platform/heap/StackFrameDepth.h" #include "platform/heap/Visitor.h" #include "platform/heap/WrapperVisitor.h" @@ -202,7 +201,6 @@ public: static void trace(Visitor*, void* self); - static void trace(InlinedGlobalMarkingVisitor, void* self); static void markWrapperNoTracing(const WrapperVisitor*, const void*); static void traceMarkedWrapper(const WrapperVisitor*, const void*); @@ -233,18 +231,6 @@ template <typename T> void TraceTrait<T>::trace(Visitor* visitor, void* self) { static_assert(WTF::IsTraceable<T>::value, "T should not be traced"); - if (visitor->isGlobalMarking()) { - // Switch to inlined global marking dispatch. - static_cast<T*>(self)->trace(InlinedGlobalMarkingVisitor( - visitor->state(), visitor->getMarkingMode())); - } else { - static_cast<T*>(self)->trace(visitor); - } -} - -template <typename T> -void TraceTrait<T>::trace(InlinedGlobalMarkingVisitor visitor, void* self) { - static_assert(WTF::IsTraceable<T>::value, "T should not be traced"); static_cast<T*>(self)->trace(visitor); }
diff --git a/third_party/WebKit/Source/platform/heap/Visitor.cpp b/third_party/WebKit/Source/platform/heap/Visitor.cpp index 9187501d..2d8f176 100644 --- a/third_party/WebKit/Source/platform/heap/Visitor.cpp +++ b/third_party/WebKit/Source/platform/heap/Visitor.cpp
@@ -5,8 +5,8 @@ #include "platform/heap/Visitor.h" #include "platform/heap/BlinkGC.h" -#include "platform/heap/MarkingVisitor.h" #include "platform/heap/ThreadState.h" +#include "platform/heap/VisitorImpl.h" #include "wtf/PtrUtil.h" #include <memory> @@ -14,11 +14,11 @@ std::unique_ptr<Visitor> Visitor::create(ThreadState* state, VisitorMarkingMode mode) { - return WTF::makeUnique<MarkingVisitor>(state, mode); + return WTF::makeUnique<Visitor>(state, mode); } Visitor::Visitor(ThreadState* state, VisitorMarkingMode markingMode) - : VisitorHelper(state, markingMode) { + : m_state(state), m_markingMode(markingMode) { // See ThreadState::runScheduledGC() why we need to already be in a // GCForbiddenScope before any safe point is entered. DCHECK(state->isGCForbidden()); @@ -29,4 +29,8 @@ Visitor::~Visitor() {} +void Visitor::markNoTracingCallback(Visitor* visitor, void* object) { + visitor->markNoTracing(object); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/platform/heap/Visitor.h b/third_party/WebKit/Source/platform/heap/Visitor.h index aacbce11..b4036f2 100644 --- a/third_party/WebKit/Source/platform/heap/Visitor.h +++ b/third_party/WebKit/Source/platform/heap/Visitor.h
@@ -45,7 +45,6 @@ template <typename T> class GarbageCollected; class HeapObjectHeader; -class InlinedGlobalMarkingVisitor; template <typename T> class TraceTrait; class ThreadState; @@ -69,62 +68,24 @@ } }; -#define DECLARE_TRACE_IMPL(maybevirtual) \ - public: \ - maybevirtual void trace(blink::Visitor*); \ - maybevirtual void trace(blink::InlinedGlobalMarkingVisitor); \ - \ - private: \ - template <typename VisitorDispatcher> \ - void traceImpl(VisitorDispatcher); \ - \ - public: -#define DEFINE_TRACE(T) \ - void T::trace(blink::Visitor* visitor) { traceImpl(visitor); } \ - void T::trace(blink::InlinedGlobalMarkingVisitor visitor) { \ - traceImpl(visitor); \ - } \ - template <typename VisitorDispatcher> \ - ALWAYS_INLINE void T::traceImpl(VisitorDispatcher visitor) +#define DECLARE_TRACE_IMPL(maybevirtual) \ + public: \ + maybevirtual void trace(blink::Visitor*); -#define DEFINE_INLINE_TRACE_IMPL(maybevirtual) \ - maybevirtual void trace(blink::Visitor* visitor) { traceImpl(visitor); } \ - maybevirtual void trace(blink::InlinedGlobalMarkingVisitor visitor) { \ - traceImpl(visitor); \ - } \ - template <typename VisitorDispatcher> \ - inline void traceImpl(VisitorDispatcher visitor) +#define DECLARE_TRACE_AFTER_DISPATCH() \ + public: \ + void traceAfterDispatch(blink::Visitor*); -#define DECLARE_TRACE_AFTER_DISPATCH() \ - public: \ - void traceAfterDispatch(blink::Visitor*); \ - void traceAfterDispatch(blink::InlinedGlobalMarkingVisitor); \ - \ - private: \ - template <typename VisitorDispatcher> \ - void traceAfterDispatchImpl(VisitorDispatcher); \ - \ - public: +#define DEFINE_TRACE(T) void T::trace(blink::Visitor* visitor) -#define DEFINE_TRACE_AFTER_DISPATCH(T) \ - void T::traceAfterDispatch(blink::Visitor* visitor) { \ - traceAfterDispatchImpl(visitor); \ - } \ - void T::traceAfterDispatch(blink::InlinedGlobalMarkingVisitor visitor) { \ - traceAfterDispatchImpl(visitor); \ - } \ - template <typename VisitorDispatcher> \ - ALWAYS_INLINE void T::traceAfterDispatchImpl(VisitorDispatcher visitor) +#define DEFINE_INLINE_TRACE_IMPL(maybevirtual) \ + maybevirtual void trace(blink::Visitor* visitor) -#define DEFINE_INLINE_TRACE_AFTER_DISPATCH() \ - void traceAfterDispatch(blink::Visitor* visitor) { \ - traceAfterDispatchImpl(visitor); \ - } \ - void traceAfterDispatch(blink::InlinedGlobalMarkingVisitor visitor) { \ - traceAfterDispatchImpl(visitor); \ - } \ - template <typename VisitorDispatcher> \ - inline void traceAfterDispatchImpl(VisitorDispatcher visitor) +#define DEFINE_TRACE_AFTER_DISPATCH(T) \ + void T::traceAfterDispatch(blink::Visitor* visitor) + +#define DEFINE_INLINE_TRACE_AFTER_DISPATCH() \ + void traceAfterDispatch(blink::Visitor* visitor) #define EMPTY_MACRO_ARGUMENT @@ -155,15 +116,21 @@ GlobalMarkingWithCompaction, }; -// VisitorHelper contains common implementation of Visitor helper methods. +// Visitor is used to traverse the Blink object graph. Used for the +// marking phase of the mark-sweep garbage collector. // -// VisitorHelper avoids virtual methods by using CRTP. -// c.f. http://en.wikipedia.org/wiki/Curiously_Recurring_Template_Pattern -template <typename Derived> -class VisitorHelper { +// Pointers are marked and pushed on the marking stack by calling the +// |mark| method with the pointer as an argument. +// +// Pointers within objects are traced by calling the |trace| methods +// with the object as an argument. Tracing objects will mark all of the +// contained pointers and push them on the marking stack. +class PLATFORM_EXPORT Visitor { public: - VisitorHelper(ThreadState* state, VisitorMarkingMode markingMode) - : m_state(state), m_markingMode(markingMode) {} + static std::unique_ptr<Visitor> create(ThreadState*, VisitorMarkingMode); + + Visitor(ThreadState*, VisitorMarkingMode); + virtual ~Visitor(); // One-argument templated mark method. This uses the static type of // the argument to get the TraceTrait. By default, the mark method @@ -176,7 +143,7 @@ "T needs to be a garbage collected object"); if (!t) return; - TraceTrait<T>::mark(Derived::fromHelper(this), t); + TraceTrait<T>::mark(this, t); } // Member version of the one-argument templated trace method. @@ -223,7 +190,7 @@ template <typename T> void traceInCollection(T& t, WTF::ShouldWeakPointersBeMarkedStrongly strongify) { - HashTraits<T>::traceInCollection(Derived::fromHelper(this), t, strongify); + HashTraits<T>::traceInCollection(this, t, strongify); } // Fallback trace method for part objects to allow individual trace methods @@ -242,16 +209,7 @@ if (!vtable) return; } - TraceTrait<T>::trace(Derived::fromHelper(this), &const_cast<T&>(t)); - } - - void markNoTracing(const void* pointer) { - Derived::fromHelper(this)->mark(pointer, - reinterpret_cast<TraceCallback>(0)); - } - void markHeaderNoTracing(HeapObjectHeader* header) { - Derived::fromHelper(this)->markHeader(header, - reinterpret_cast<TraceCallback>(0)); + TraceTrait<T>::trace(this, &const_cast<T&>(t)); } // For simple cases where you just want to zero out a cell when the thing @@ -265,7 +223,7 @@ // threads are stopped during weak cell callbacks. template <typename T> void registerWeakCell(T** cell) { - Derived::fromHelper(this)->registerWeakCellWithCallback( + registerWeakCellWithCallback( reinterpret_cast<void**>( const_cast<typename std::remove_const<T>::type**>(cell)), &handleWeakCell<T>); @@ -277,63 +235,14 @@ } void registerWeakMembers(const void* object, WeakCallback callback) { - Derived::fromHelper(this)->registerWeakMembers(object, object, callback); + registerWeakMembers(object, object, callback); } - void registerBackingStoreReference(void* slot) { - if (getMarkingMode() != VisitorMarkingMode::GlobalMarkingWithCompaction) - return; - heap().registerMovingObjectReference( - reinterpret_cast<MovableReference*>(slot)); - } + inline void registerBackingStoreReference(void* slot); - void registerBackingStoreCallback(void* backingStore, - MovingObjectCallback callback, - void* callbackData) { - if (getMarkingMode() != VisitorMarkingMode::GlobalMarkingWithCompaction) - return; - heap().registerMovingObjectCallback( - reinterpret_cast<MovableReference>(backingStore), callback, - callbackData); - } - - inline ThreadState* state() const { return m_state; } - inline ThreadHeap& heap() const { return state()->heap(); } - - inline VisitorMarkingMode getMarkingMode() const { return m_markingMode; } - - inline bool isGlobalMarking() const { - return m_markingMode == VisitorMarkingMode::GlobalMarking || - m_markingMode == VisitorMarkingMode::GlobalMarkingWithCompaction; - } - - private: - template <typename T> - static void handleWeakCell(Visitor* self, void* object); - - ThreadState* const m_state; - const VisitorMarkingMode m_markingMode; -}; - -// Visitor is used to traverse the Blink object graph. Used for the -// marking phase of the mark-sweep garbage collector. -// -// Pointers are marked and pushed on the marking stack by calling the -// |mark| method with the pointer as an argument. -// -// Pointers within objects are traced by calling the |trace| methods -// with the object as an argument. Tracing objects will mark all of the -// contained pointers and push them on the marking stack. -class PLATFORM_EXPORT Visitor : public VisitorHelper<Visitor> { - public: - friend class VisitorHelper<Visitor>; - friend class InlinedGlobalMarkingVisitor; - - static std::unique_ptr<Visitor> create(ThreadState*, VisitorMarkingMode); - - virtual ~Visitor(); - - using VisitorHelper<Visitor>::mark; + inline void registerBackingStoreCallback(void* backingStore, + MovingObjectCallback, + void* callbackData); // This method marks an object and adds it to the set of objects // that should have their trace method called. Since not all @@ -341,10 +250,7 @@ // explicit argument, but we can use the templated one-argument // mark method above to automatically provide the callback // function. - virtual void mark(const void*, TraceCallback) = 0; - - // Used to mark objects during conservative scanning. - virtual void markHeader(HeapObjectHeader*, TraceCallback) = 0; + inline void mark(const void* objectPointer, TraceCallback); // Used to delay the marking of objects until the usual marking // including emphemeron iteration is done. This is used to delay @@ -353,7 +259,7 @@ // object. If collection backings are reachable from other // locations we strongify them to avoid issues with iterators and // weak processing. - virtual void registerDelayedMarkNoTracing(const void*) = 0; + inline void registerDelayedMarkNoTracing(const void* pointer); // If the object calls this during the regular trace callback, then the // WeakCallback argument may be called later, when the strong roots @@ -375,27 +281,48 @@ // pointed to belong to the same thread as the object receiving // the weak callback. Since other threads have been resumed the // mark bits are not valid for objects from other threads. - virtual void registerWeakMembers(const void*, const void*, WeakCallback) = 0; - using VisitorHelper<Visitor>::registerWeakMembers; + inline void registerWeakMembers(const void* closure, + const void* pointer, + WeakCallback); - virtual void registerWeakTable(const void*, - EphemeronCallback, - EphemeronCallback) = 0; + inline void registerWeakTable(const void* closure, + EphemeronCallback iterationCallback, + EphemeronCallback iterationDoneCallback); + #if DCHECK_IS_ON() - virtual bool weakTableRegistered(const void*) = 0; + inline bool weakTableRegistered(const void* closure); #endif - virtual bool ensureMarked(const void*) = 0; + inline bool ensureMarked(const void* pointer); - virtual void registerWeakCellWithCallback(void**, WeakCallback) = 0; + inline void registerWeakCellWithCallback(void** cell, WeakCallback); - protected: - Visitor(ThreadState*, VisitorMarkingMode); + inline void markNoTracing(const void* pointer) { + mark(pointer, reinterpret_cast<TraceCallback>(0)); + } + + inline void markHeaderNoTracing(HeapObjectHeader*); + + // Used to mark objects during conservative scanning. + inline void markHeader(HeapObjectHeader*, + const void* objectPointer, + TraceCallback); + + inline void markHeader(HeapObjectHeader*, TraceCallback); + + inline ThreadState* state() const { return m_state; } + inline ThreadHeap& heap() const { return state()->heap(); } + + inline VisitorMarkingMode getMarkingMode() const { return m_markingMode; } private: - static Visitor* fromHelper(VisitorHelper<Visitor>* helper) { - return static_cast<Visitor*>(helper); - } + template <typename T> + static void handleWeakCell(Visitor* self, void*); + + static void markNoTracingCallback(Visitor*, void*); + + ThreadState* const m_state; + const VisitorMarkingMode m_markingMode; }; } // namespace blink
diff --git a/third_party/WebKit/Source/platform/heap/VisitorImpl.h b/third_party/WebKit/Source/platform/heap/VisitorImpl.h new file mode 100644 index 0000000..f2e90e00 --- /dev/null +++ b/third_party/WebKit/Source/platform/heap/VisitorImpl.h
@@ -0,0 +1,137 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef VisitorImpl_h +#define VisitorImpl_h + +#include "platform/heap/Heap.h" +#include "platform/heap/ThreadState.h" +#include "platform/heap/Visitor.h" +#include "wtf/Allocator.h" + +namespace blink { + +inline void Visitor::markHeader(HeapObjectHeader* header, + const void* objectPointer, + TraceCallback callback) { + DCHECK(header); + DCHECK(objectPointer); + + // If you hit this DCHECK, it means that there is a dangling pointer + // from a live thread heap to a dead thread heap. We must eliminate + // the dangling pointer. + // Release builds don't have the DCHECK, but it is OK because + // release builds will crash in the following header->isMarked() + // because all the entries of the orphaned arenas are zapped. + DCHECK(!pageFromObject(objectPointer)->orphaned()); + + if (header->isMarked()) + return; + + DCHECK(ThreadState::current()->isInGC()); + DCHECK(getMarkingMode() != VisitorMarkingMode::WeakProcessing); + + // A GC should only mark the objects that belong in its heap. + DCHECK(&pageFromObject(objectPointer)->arena()->getThreadState()->heap() == + &heap()); + + header->mark(); + + if (callback) + heap().pushTraceCallback(const_cast<void*>(objectPointer), callback); +} + +inline void Visitor::markHeader(HeapObjectHeader* header, + TraceCallback callback) { + markHeader(header, header->payload(), callback); +} + +inline void Visitor::mark(const void* objectPointer, TraceCallback callback) { + if (!objectPointer) + return; + HeapObjectHeader* header = HeapObjectHeader::fromPayload(objectPointer); + markHeader(header, header->payload(), callback); +} + +inline void Visitor::markHeaderNoTracing(HeapObjectHeader* header) { + markHeader(header, header->payload(), reinterpret_cast<TraceCallback>(0)); +} + +inline void Visitor::registerDelayedMarkNoTracing(const void* objectPointer) { + DCHECK(getMarkingMode() != VisitorMarkingMode::WeakProcessing); + heap().pushPostMarkingCallback(const_cast<void*>(objectPointer), + &markNoTracingCallback); +} + +inline void Visitor::registerWeakMembers(const void* closure, + const void* objectPointer, + WeakCallback callback) { + DCHECK(getMarkingMode() != VisitorMarkingMode::WeakProcessing); + // We don't want to run weak processings when taking a snapshot. + if (getMarkingMode() == VisitorMarkingMode::SnapshotMarking) + return; + heap().pushThreadLocalWeakCallback( + const_cast<void*>(closure), const_cast<void*>(objectPointer), callback); +} + +inline void Visitor::registerWeakTable( + const void* closure, + EphemeronCallback iterationCallback, + EphemeronCallback iterationDoneCallback) { + DCHECK(getMarkingMode() != VisitorMarkingMode::WeakProcessing); + heap().registerWeakTable(const_cast<void*>(closure), iterationCallback, + iterationDoneCallback); +} + +#if DCHECK_IS_ON() +inline bool Visitor::weakTableRegistered(const void* closure) { + return heap().weakTableRegistered(closure); +} +#endif + +inline bool Visitor::ensureMarked(const void* objectPointer) { + if (!objectPointer) + return false; + + HeapObjectHeader* header = HeapObjectHeader::fromPayload(objectPointer); + if (header->isMarked()) + return false; +#if DCHECK_IS_ON() + markNoTracing(objectPointer); +#else + // Inline what the above markNoTracing() call expands to, + // so as to make sure that we do get all the benefits (asserts excepted.) + header->mark(); +#endif + return true; +} + +inline void Visitor::registerWeakCellWithCallback(void** cell, + WeakCallback callback) { + DCHECK(getMarkingMode() != VisitorMarkingMode::WeakProcessing); + // We don't want to run weak processings when taking a snapshot. + if (getMarkingMode() == VisitorMarkingMode::SnapshotMarking) + return; + heap().pushGlobalWeakCallback(cell, callback); +} + +inline void Visitor::registerBackingStoreReference(void* slot) { + if (getMarkingMode() != VisitorMarkingMode::GlobalMarkingWithCompaction) + return; + heap().registerMovingObjectReference( + reinterpret_cast<MovableReference*>(slot)); +} + +inline void Visitor::registerBackingStoreCallback(void* backingStore, + MovingObjectCallback callback, + void* callbackData) { + if (getMarkingMode() != VisitorMarkingMode::GlobalMarkingWithCompaction) + return; + heap().registerMovingObjectCallback( + reinterpret_cast<MovableReference>(backingStore), callback, callbackData); +} + +} // namespace blink + +#endif
diff --git a/third_party/WebKit/Source/web/OpenedFrameTracker.cpp b/third_party/WebKit/Source/web/OpenedFrameTracker.cpp index 4efde8b..1cb1f9c 100644 --- a/third_party/WebKit/Source/web/OpenedFrameTracker.cpp +++ b/third_party/WebKit/Source/web/OpenedFrameTracker.cpp
@@ -45,8 +45,5 @@ void OpenedFrameTracker::traceFrames(Visitor* visitor) { traceFramesImpl(visitor); } -void OpenedFrameTracker::traceFrames(InlinedGlobalMarkingVisitor visitor) { - traceFramesImpl(visitor); -} } // namespace blink
diff --git a/third_party/WebKit/Source/web/OpenedFrameTracker.h b/third_party/WebKit/Source/web/OpenedFrameTracker.h index 3aec0e5..09c2e0b 100644 --- a/third_party/WebKit/Source/web/OpenedFrameTracker.h +++ b/third_party/WebKit/Source/web/OpenedFrameTracker.h
@@ -11,7 +11,6 @@ namespace blink { class Visitor; -class InlinedGlobalMarkingVisitor; class WebFrame; // Small helper class to track the set of frames that a WebFrame has opened. @@ -33,7 +32,6 @@ void transferTo(WebFrame*); void traceFrames(Visitor*); - void traceFrames(InlinedGlobalMarkingVisitor); private: template <typename VisitorDispatcher>
diff --git a/third_party/WebKit/Source/web/WebFrame.cpp b/third_party/WebKit/Source/web/WebFrame.cpp index a12cb5d..61f14303 100644 --- a/third_party/WebKit/Source/web/WebFrame.cpp +++ b/third_party/WebKit/Source/web/WebFrame.cpp
@@ -342,7 +342,6 @@ } DEFINE_VISITOR_METHOD(Visitor*) -DEFINE_VISITOR_METHOD(InlinedGlobalMarkingVisitor) #undef DEFINE_VISITOR_METHOD
diff --git a/third_party/WebKit/Source/web/WebMemoryStatistics.cpp b/third_party/WebKit/Source/web/WebMemoryStatistics.cpp index f722a18c..540b0e61 100644 --- a/third_party/WebKit/Source/web/WebMemoryStatistics.cpp +++ b/third_party/WebKit/Source/web/WebMemoryStatistics.cpp
@@ -4,7 +4,7 @@ #include "public/web/WebMemoryStatistics.h" -#include "platform/heap/Heap.h" +#include "platform/heap/Handle.h" #include "wtf/allocator/Partitions.h" namespace blink {
diff --git a/third_party/WebKit/public/web/WebFrame.h b/third_party/WebKit/public/web/WebFrame.h index c12e89f..96dd9ad0 100644 --- a/third_party/WebKit/public/web/WebFrame.h +++ b/third_party/WebKit/public/web/WebFrame.h
@@ -439,9 +439,7 @@ bool inShadowTree() const { return m_scope == WebTreeScopeType::Shadow; } static void traceFrames(Visitor*, WebFrame*); - static void traceFrames(InlinedGlobalMarkingVisitor, WebFrame*); void clearWeakFrames(Visitor*); - void clearWeakFrames(InlinedGlobalMarkingVisitor); #endif protected: @@ -460,7 +458,6 @@ friend class WebFrameTest; static void traceFrame(Visitor*, WebFrame*); - static void traceFrame(InlinedGlobalMarkingVisitor, WebFrame*); static bool isFrameAlive(const WebFrame*); template <typename VisitorDispatcher>
diff --git a/third_party/libxslt/README.chromium b/third_party/libxslt/README.chromium index 4515315..d76cd6eb 100644 --- a/third_party/libxslt/README.chromium +++ b/third_party/libxslt/README.chromium
@@ -16,6 +16,9 @@ - Apply patch contributed here: https://bugs.chromium.org/p/chromium/issues/detail?id=583171#c17 +- Apply patch contributed upstream, details here: + https://crbug.com/676623#c18 + To import a new version: On Linux, get the latest tar via libxml.org and extract and replace
diff --git a/third_party/libxslt/libxslt/transform.c b/third_party/libxslt/libxslt/transform.c index 519133f..02bff34 100644 --- a/third_party/libxslt/libxslt/transform.c +++ b/third_party/libxslt/libxslt/transform.c
@@ -813,13 +813,32 @@ return(target); if (ctxt->lasttext == target->content) { + int minSize; - if (ctxt->lasttuse + len >= ctxt->lasttsize) { + /* Check for integer overflow accounting for NUL terminator. */ + if (len >= INT_MAX - ctxt->lasttuse) { + xsltTransformError(ctxt, NULL, target, + "xsltCopyText: text allocation failed\n"); + return(NULL); + } + minSize = ctxt->lasttuse + len + 1; + + if (ctxt->lasttsize < minSize) { xmlChar *newbuf; int size; + int extra; - size = ctxt->lasttsize + len + 100; - size *= 2; + /* Double buffer size but increase by at least 100 bytes. */ + extra = minSize < 100 ? 100 : minSize; + + /* Check for integer overflow. */ + if (extra > INT_MAX - ctxt->lasttsize) { + size = INT_MAX; + } + else { + size = ctxt->lasttsize + extra; + } + newbuf = (xmlChar *) xmlRealloc(target->content,size); if (newbuf == NULL) { xsltTransformError(ctxt, NULL, target,
diff --git a/third_party/libxslt/libxslt/xsltInternals.h b/third_party/libxslt/libxslt/xsltInternals.h index 060b1783..5ad1771 100644 --- a/third_party/libxslt/libxslt/xsltInternals.h +++ b/third_party/libxslt/libxslt/xsltInternals.h
@@ -1754,8 +1754,8 @@ * Speed optimization when coalescing text nodes */ const xmlChar *lasttext; /* last text node content */ - unsigned int lasttsize; /* last text node size */ - unsigned int lasttuse; /* last text node use */ + int lasttsize; /* last text node size */ + int lasttuse; /* last text node use */ /* * Per Context Debugging */
diff --git a/ui/display/manager/chromeos/touch_transform_controller.cc b/ui/display/manager/chromeos/touch_transform_controller.cc index 82f8212..99f54ec8 100644 --- a/ui/display/manager/chromeos/touch_transform_controller.cc +++ b/ui/display/manager/chromeos/touch_transform_controller.cc
@@ -152,6 +152,13 @@ return ctm; } +DisplayIdList GetCurrentDisplayIdList(const DisplayManager* display_manager) { + DCHECK(display_manager->num_connected_displays()); + if (display_manager->num_connected_displays() == 1) + return DisplayIdList{display_manager->first_display_id()}; + return display_manager->GetCurrentDisplayIdList(); +} + } // namespace // This is to compute the scale ratio for the TouchEvent's radius. The @@ -308,65 +315,47 @@ void TouchTransformController::UpdateTouchTransforms() const { ui::DeviceDataManager::GetInstance()->ClearTouchDeviceAssociations(); - - // Display IDs and ManagedDisplayInfo for mirror or extended mode. - int64_t display1_id = kInvalidDisplayId; - int64_t display2_id = kInvalidDisplayId; - ManagedDisplayInfo display1; - ManagedDisplayInfo display2; - // Display ID and ManagedDisplayInfo for single display mode. - int64_t single_display_id = kInvalidDisplayId; - ManagedDisplayInfo single_display; - - if (display_manager_->num_connected_displays() == 0) { + if (display_manager_->num_connected_displays() == 0) return; - } else if (display_manager_->num_connected_displays() == 1 || - display_manager_->IsInUnifiedMode()) { - single_display_id = display_manager_->first_display_id(); - DCHECK(single_display_id != kInvalidDisplayId); - single_display = display_manager_->GetDisplayInfo(single_display_id); - UpdateTouchRadius(single_display); - } else { - DisplayIdList list = display_manager_->GetCurrentDisplayIdList(); - display1_id = list[0]; - display2_id = list[1]; - DCHECK(display1_id != kInvalidDisplayId && - display2_id != kInvalidDisplayId); - display1 = display_manager_->GetDisplayInfo(display1_id); - display2 = display_manager_->GetDisplayInfo(display2_id); - UpdateTouchRadius(display1); - UpdateTouchRadius(display2); + + DisplayIdList display_id_list = GetCurrentDisplayIdList(display_manager_); + DCHECK(display_id_list.size()); + + DisplayInfoList display_info_list; + + for (int64_t display_id : display_id_list) { + DCHECK(display_id != kInvalidDisplayId); + display_info_list.push_back(display_manager_->GetDisplayInfo(display_id)); + UpdateTouchRadius(display_info_list.back()); } if (display_manager_->IsInMirrorMode()) { - int64_t primary_display_id = Screen::GetScreen()->GetPrimaryDisplay().id(); - if (display_manager_->SoftwareMirroringEnabled()) { - // In extended but software mirroring mode, there is a WindowTreeHost for - // each display, but all touches are forwarded to the primary root + std::size_t primary_display_id_index = + std::distance(display_id_list.begin(), + std::find(display_id_list.begin(), display_id_list.end(), + Screen::GetScreen()->GetPrimaryDisplay().id())); + + for (std::size_t index = 0; index < display_id_list.size(); index++) { + // In extended but software mirroring mode, there is a WindowTreeHost + // for each display, but all touches are forwarded to the primary root // window's WindowTreeHost. - ManagedDisplayInfo target_display = - primary_display_id == display1_id ? display1 : display2; - UpdateTouchTransform(target_display.id(), display1, target_display); - UpdateTouchTransform(target_display.id(), display2, target_display); - } else { - // In mirror mode, there is just one WindowTreeHost and two displays. Make - // the WindowTreeHost accept touch events from both displays. - UpdateTouchTransform(primary_display_id, display1, display1); - UpdateTouchTransform(primary_display_id, display2, display2); + // In mirror mode, there is just one WindowTreeHost and two displays. + // Make the WindowTreeHost accept touch events from both displays. + std::size_t touch_display_index = + display_manager_->SoftwareMirroringEnabled() + ? primary_display_id_index + : index; + UpdateTouchTransform(display_id_list[primary_display_id_index], + display_info_list[index], + display_info_list[touch_display_index]); } return; } - if (display_manager_->num_connected_displays() > 1) { - // In actual extended mode, each display is associated with one - // WindowTreeHost. - UpdateTouchTransform(display1_id, display1, display1); - UpdateTouchTransform(display2_id, display2, display2); - return; + for (std::size_t index = 0; index < display_id_list.size(); index++) { + UpdateTouchTransform(display_id_list[index], display_info_list[index], + display_info_list[index]); } - - // Single display mode. The WindowTreeHost has one associated display id. - UpdateTouchTransform(single_display_id, single_display, single_display); } void TouchTransformController::SetForCalibration(bool is_calibrating) {