diff --git a/DEPS b/DEPS index f432a98b..1ce7aea 100644 --- a/DEPS +++ b/DEPS
@@ -235,7 +235,7 @@ Var('chromium_git') + '/native_client/src/third_party/scons-2.0.1.git' + '@' + '1c1550e17fc26355d08627fbdec13d8291227067', 'src/third_party/webrtc': - Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + 'e9cc1fe00713026a257a65e0fb958d6f3573aec7', # commit position 18649 + Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + 'f91805c46d7a5067e3b4ce2bd066ca504dc167f3', # commit position 18617 'src/third_party/openmax_dl': Var('chromium_git') + '/external/webrtc/deps/third_party/openmax.git' + '@' + Var('openmax_dl_revision'),
diff --git a/base/files/important_file_writer.cc b/base/files/important_file_writer.cc index 0696c96..688b6d2 100644 --- a/base/files/important_file_writer.cc +++ b/base/files/important_file_writer.cc
@@ -19,6 +19,7 @@ #include "base/files/file_util.h" #include "base/logging.h" #include "base/macros.h" +#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_macros.h" #include "base/numerics/safe_conversions.h" #include "base/strings/string_number_conversions.h" @@ -49,10 +50,34 @@ TEMP_FILE_FAILURE_MAX }; -void LogFailure(const FilePath& path, TempFileFailure failure_code, +// Helper function to write samples to a histogram with a dynamically assigned +// histogram name. Works with different error code types convertible to int +// which is the actual argument type of UmaHistogramExactLinear. +template <typename SampleType> +void UmaHistogramExactLinearWithSuffix(const char* histogram_name, + StringPiece histogram_suffix, + SampleType add_sample, + SampleType max_sample) { + static_assert(std::is_convertible<SampleType, int>::value, + "SampleType should be convertible to int"); + DCHECK(histogram_name); + std::string histogram_full_name(histogram_name); + if (!histogram_suffix.empty()) { + histogram_full_name.append("."); + histogram_full_name.append(histogram_suffix.data(), + histogram_suffix.length()); + } + UmaHistogramExactLinear(histogram_full_name, static_cast<int>(add_sample), + static_cast<int>(max_sample)); +} + +void LogFailure(const FilePath& path, + StringPiece histogram_suffix, + TempFileFailure failure_code, StringPiece message) { - UMA_HISTOGRAM_ENUMERATION("ImportantFile.TempFileFailures", failure_code, - TEMP_FILE_FAILURE_MAX); + UmaHistogramExactLinearWithSuffix("ImportantFile.TempFileFailures", + histogram_suffix, failure_code, + TEMP_FILE_FAILURE_MAX); DPLOG(WARNING) << "temp file failure: " << path.value() << " : " << message; } @@ -62,21 +87,43 @@ const FilePath& path, std::unique_ptr<std::string> data, Closure before_write_callback, - Callback<void(bool success)> after_write_callback) { + Callback<void(bool success)> after_write_callback, + const std::string& histogram_suffix) { if (!before_write_callback.is_null()) before_write_callback.Run(); - bool result = ImportantFileWriter::WriteFileAtomically(path, *data); + bool result = + ImportantFileWriter::WriteFileAtomically(path, *data, histogram_suffix); if (!after_write_callback.is_null()) after_write_callback.Run(result); } +base::File::Error GetLastFileError() { +#if defined(OS_WIN) + return base::File::OSErrorToFileError(::GetLastError()); +#elif defined(OS_POSIX) + return base::File::OSErrorToFileError(errno); +#else + return base::File::FILE_OK; +#endif +} + +void DeleteTmpFile(const FilePath& tmp_file_path, + StringPiece histogram_suffix) { + if (!DeleteFile(tmp_file_path, false)) { + UmaHistogramExactLinearWithSuffix("ImportantFile.FileDeleteError", + histogram_suffix, -GetLastFileError(), + -base::File::FILE_ERROR_MAX); + } +} + } // namespace // static bool ImportantFileWriter::WriteFileAtomically(const FilePath& path, - StringPiece data) { + StringPiece data, + StringPiece histogram_suffix) { #if defined(OS_CHROMEOS) // On Chrome OS, chrome gets killed when it cannot finish shutdown quickly, // and this function seems to be one of the slowest shutdown steps. @@ -97,13 +144,21 @@ // is securely created. FilePath tmp_file_path; if (!CreateTemporaryFileInDir(path.DirName(), &tmp_file_path)) { - LogFailure(path, FAILED_CREATING, "could not create temporary file"); + UmaHistogramExactLinearWithSuffix("ImportantFile.FileCreateError", + histogram_suffix, -GetLastFileError(), + -base::File::FILE_ERROR_MAX); + LogFailure(path, histogram_suffix, FAILED_CREATING, + "could not create temporary file"); return false; } File tmp_file(tmp_file_path, File::FLAG_OPEN | File::FLAG_WRITE); if (!tmp_file.IsValid()) { - LogFailure(path, FAILED_OPENING, "could not open temporary file"); + UmaHistogramExactLinearWithSuffix( + "ImportantFile.FileOpenError", histogram_suffix, + -tmp_file.error_details(), -base::File::FILE_ERROR_MAX); + LogFailure(path, histogram_suffix, FAILED_OPENING, + "could not open temporary file"); DeleteFile(tmp_file_path, false); return false; } @@ -111,25 +166,35 @@ // If this fails in the wild, something really bad is going on. const int data_length = checked_cast<int32_t>(data.length()); int bytes_written = tmp_file.Write(0, data.data(), data_length); + if (bytes_written < data_length) { + UmaHistogramExactLinearWithSuffix("ImportantFile.FileWriteError", + histogram_suffix, -GetLastFileError(), + -base::File::FILE_ERROR_MAX); + } bool flush_success = tmp_file.Flush(); tmp_file.Close(); if (bytes_written < data_length) { - LogFailure(path, FAILED_WRITING, "error writing, bytes_written=" + - IntToString(bytes_written)); - DeleteFile(tmp_file_path, false); + LogFailure(path, histogram_suffix, FAILED_WRITING, + "error writing, bytes_written=" + IntToString(bytes_written)); + DeleteTmpFile(tmp_file_path, histogram_suffix); return false; } if (!flush_success) { - LogFailure(path, FAILED_FLUSHING, "error flushing"); - DeleteFile(tmp_file_path, false); + LogFailure(path, histogram_suffix, FAILED_FLUSHING, "error flushing"); + DeleteTmpFile(tmp_file_path, histogram_suffix); return false; } - if (!ReplaceFile(tmp_file_path, path, nullptr)) { - LogFailure(path, FAILED_RENAMING, "could not rename temporary file"); - DeleteFile(tmp_file_path, false); + base::File::Error replace_file_error = base::File::FILE_OK; + if (!ReplaceFile(tmp_file_path, path, &replace_file_error)) { + UmaHistogramExactLinearWithSuffix("ImportantFile.FileRenameError", + histogram_suffix, -replace_file_error, + -base::File::FILE_ERROR_MAX); + LogFailure(path, histogram_suffix, FAILED_RENAMING, + "could not rename temporary file"); + DeleteTmpFile(tmp_file_path, histogram_suffix); return false; } @@ -138,19 +203,23 @@ ImportantFileWriter::ImportantFileWriter( const FilePath& path, - scoped_refptr<SequencedTaskRunner> task_runner) + scoped_refptr<SequencedTaskRunner> task_runner, + const char* histogram_suffix) : ImportantFileWriter(path, std::move(task_runner), - kDefaultCommitInterval) {} + kDefaultCommitInterval, + histogram_suffix) {} ImportantFileWriter::ImportantFileWriter( const FilePath& path, scoped_refptr<SequencedTaskRunner> task_runner, - TimeDelta interval) + TimeDelta interval, + const char* histogram_suffix) : path_(path), task_runner_(std::move(task_runner)), serializer_(nullptr), commit_interval_(interval), + histogram_suffix_(histogram_suffix ? histogram_suffix : ""), weak_factory_(this) { DCHECK(task_runner_); } @@ -178,7 +247,7 @@ Closure task = AdaptCallbackForRepeating( BindOnce(&WriteScopedStringToFileAtomically, path_, std::move(data), std::move(before_next_write_callback_), - std::move(after_next_write_callback_))); + std::move(after_next_write_callback_), histogram_suffix_)); if (!task_runner_->PostTask(FROM_HERE, MakeCriticalClosure(task))) { // Posting the task to background message loop is not expected
diff --git a/base/files/important_file_writer.h b/base/files/important_file_writer.h index 5fcd515..3d04eec 100644 --- a/base/files/important_file_writer.h +++ b/base/files/important_file_writer.h
@@ -53,7 +53,9 @@ // Save |data| to |path| in an atomic manner. Blocks and writes data on the // current thread. Does not guarantee file integrity across system crash (see // the class comment above). - static bool WriteFileAtomically(const FilePath& path, StringPiece data); + static bool WriteFileAtomically(const FilePath& path, + StringPiece data, + StringPiece histogram_suffix = StringPiece()); // Initialize the writer. // |path| is the name of file to write. @@ -61,12 +63,14 @@ // execute file I/O operations. // All non-const methods, ctor and dtor must be called on the same thread. ImportantFileWriter(const FilePath& path, - scoped_refptr<SequencedTaskRunner> task_runner); + scoped_refptr<SequencedTaskRunner> task_runner, + const char* histogram_suffix = nullptr); // Same as above, but with a custom commit interval. ImportantFileWriter(const FilePath& path, scoped_refptr<SequencedTaskRunner> task_runner, - TimeDelta interval); + TimeDelta interval, + const char* histogram_suffix = nullptr); // You have to ensure that there are no pending writes at the moment // of destruction. @@ -143,6 +147,9 @@ // Time delta after which scheduled data will be written to disk. const TimeDelta commit_interval_; + // Custom histogram suffix. + const std::string histogram_suffix_; + SEQUENCE_CHECKER(sequence_checker_); WeakPtrFactory<ImportantFileWriter> weak_factory_;
diff --git a/base/files/important_file_writer_unittest.cc b/base/files/important_file_writer_unittest.cc index 511f5fb..e5afedb 100644 --- a/base/files/important_file_writer_unittest.cc +++ b/base/files/important_file_writer_unittest.cc
@@ -15,6 +15,7 @@ #include "base/memory/ptr_util.h" #include "base/run_loop.h" #include "base/single_thread_task_runner.h" +#include "base/test/histogram_tester.h" #include "base/threading/thread.h" #include "base/threading/thread_task_runner_handle.h" #include "base/time/time.h" @@ -325,4 +326,25 @@ EXPECT_FALSE(PathExists(writer.path())); } +TEST_F(ImportantFileWriterTest, WriteFileAtomicallyHistogramSuffixTest) { + base::HistogramTester histogram_tester; + EXPECT_FALSE(PathExists(file_)); + EXPECT_TRUE(ImportantFileWriter::WriteFileAtomically(file_, "baz", "test")); + EXPECT_TRUE(PathExists(file_)); + EXPECT_EQ("baz", GetFileContent(file_)); + histogram_tester.ExpectTotalCount("ImportantFile.FileCreateError", 0); + histogram_tester.ExpectTotalCount("ImportantFile.FileCreateError.test", 0); + + FilePath invalid_file_ = FilePath().AppendASCII("bad/../path"); + EXPECT_FALSE(PathExists(invalid_file_)); + EXPECT_FALSE( + ImportantFileWriter::WriteFileAtomically(invalid_file_, nullptr)); + histogram_tester.ExpectTotalCount("ImportantFile.FileCreateError", 1); + histogram_tester.ExpectTotalCount("ImportantFile.FileCreateError.test", 0); + EXPECT_FALSE( + ImportantFileWriter::WriteFileAtomically(invalid_file_, nullptr, "test")); + histogram_tester.ExpectTotalCount("ImportantFile.FileCreateError", 1); + histogram_tester.ExpectTotalCount("ImportantFile.FileCreateError.test", 1); +} + } // namespace base
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java b/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java index e11d384..df66bbf 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java
@@ -5,14 +5,11 @@ package org.chromium.chrome.browser.gsa; import android.annotation.SuppressLint; -import android.app.ActivityManager; import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.content.ServiceConnection; -import android.os.AsyncTask; import android.os.Bundle; -import android.os.Debug; import android.os.Handler; import android.os.IBinder; import android.os.Message; @@ -22,15 +19,10 @@ import android.util.Log; import org.chromium.base.Callback; -import org.chromium.base.ContextUtils; -import org.chromium.base.TraceEvent; -import org.chromium.base.VisibleForTesting; import org.chromium.base.annotations.SuppressFBWarnings; import org.chromium.base.metrics.RecordHistogram; import org.chromium.chrome.browser.AppHooks; -import java.util.List; - /** * A simple client that connects and talks to the GSAService using Messages. */ @@ -41,8 +33,7 @@ * Constants for gsa communication. These should not change without corresponding changes on the * service side in GSA. */ - @VisibleForTesting - static final String GSA_SERVICE = "com.google.android.ssb.action.SSB_SERVICE"; + private static final String GSA_SERVICE = "com.google.android.ssb.action.SSB_SERVICE"; public static final int REQUEST_REGISTER_CLIENT = 2; public static final int RESPONSE_UPDATE_SSB = 3; @@ -52,16 +43,12 @@ public static final String KEY_GSA_SUPPORTS_BROADCAST = "ssb_service:chrome_holds_account_update_permission"; - @VisibleForTesting - static final int INVALID_PSS = -1; - static final String ACCOUNT_CHANGE_HISTOGRAM = "Search.GsaAccountChangeNotificationSource"; // For the histogram above. Append-only. static final int ACCOUNT_CHANGE_SOURCE_SERVICE = 0; static final int ACCOUNT_CHANGE_SOURCE_BROADCAST = 1; static final int ACCOUNT_CHANGE_SOURCE_COUNT = 2; - private static boolean sHasRecordedPss; /** Messenger to handle incoming messages from the service */ private final Messenger mMessenger; private final IncomingHandler mHandler; @@ -72,7 +59,6 @@ /** Messenger for communicating with service. */ private Messenger mService; - private ComponentName mComponentName; /** * Handler of incoming messages from service. @@ -92,80 +78,12 @@ String account = mGsaHelper.getGSAAccountFromState(bundle.getByteArray(KEY_GSA_STATE)); RecordHistogram.recordEnumeratedHistogram(ACCOUNT_CHANGE_HISTOGRAM, ACCOUNT_CHANGE_SOURCE_SERVICE, ACCOUNT_CHANGE_SOURCE_COUNT); - GSAState.getInstance(mContext.getApplicationContext()).setGsaAccount(account); - if (sHasRecordedPss) { - if (mOnMessageReceived != null) mOnMessageReceived.onResult(bundle); - return; - } - - // Getting the PSS for the GSA service process can be long, don't block the UI thread on - // that. Also, don't process the callback before the PSS is known, since the callback - // can lead to a service disconnect, which can lead to the framework killing the - // process. Hence an AsyncTask (long operation), and processing the callback in - // onPostExecute() (don't disconnect before). - sHasRecordedPss = true; - new AsyncTask<Void, Void, Integer>() { - @Override - protected Integer doInBackground(Void... params) { - TraceEvent.begin("GSAServiceClient.getPssForservice"); - try { - // Looking for the service process is done by component name, which is - // inefficient. We really want the PID, which is only accessible from within - // a Binder transaction. Since the service connection is Messenger-based, - // the calls are not processed from a Binder thread. The alternatives are: - // 1. Override methods in the framework to append the calling PID to the - // Message. - // 2. Usse msg.callingUid to narrow down the search. - // - // (1) is dirty (and brittle), and (2) only works on L+, and still requires - // to get the full list of services from ActivityManager. - return getPssForService(mComponentName); - } finally { - TraceEvent.end("GSAServiceClient.getPssForservice"); - } - } - - @Override - protected void onPostExecute(Integer pssInKB) { - if (pssInKB != INVALID_PSS) { - RecordHistogram.recordMemoryKBHistogram( - "Search.GsaProcessMemoryPss", pssInKB); - } - if (mOnMessageReceived != null) mOnMessageReceived.onResult(bundle); - } - }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); + GSAState.getInstance(mContext).setGsaAccount(account); + if (mOnMessageReceived != null) mOnMessageReceived.onResult(bundle); } } /** - * Get the PSS used by the process hosting a service. - * - * @param packageName Package name of the service to search for. - * @return the PSS in kB of the process hosting a service, or INVALID_PSS. - */ - @VisibleForTesting - static int getPssForService(ComponentName componentName) { - if (componentName == null) return INVALID_PSS; - Context context = ContextUtils.getApplicationContext(); - ActivityManager activityManager = - (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE); - List<ActivityManager.RunningServiceInfo> services = - activityManager.getRunningServices(1000); - if (services == null) return INVALID_PSS; - int pid = -1; - for (ActivityManager.RunningServiceInfo info : services) { - if (componentName.equals(info.service)) { - pid = info.pid; - break; - } - } - if (pid == -1) return INVALID_PSS; - Debug.MemoryInfo infos[] = activityManager.getProcessMemoryInfo(new int[] {pid}); - if (infos == null || infos.length == 0) return INVALID_PSS; - return infos[0].getTotalPss(); - } - - /** * Constructs an instance of this class. * * @param context Appliation context. @@ -236,7 +154,6 @@ if (mContext == null) return; mService = new Messenger(service); - mComponentName = name; try { Message registerClientMessage = Message.obtain( null, REQUEST_REGISTER_CLIENT);
diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni index e0b804ed..292189ab 100644 --- a/chrome/android/java_sources.gni +++ b/chrome/android/java_sources.gni
@@ -1430,7 +1430,6 @@ "javatests/src/org/chromium/chrome/browser/gcore/MockConnectedTask.java", "javatests/src/org/chromium/chrome/browser/gcore/MockConnectedTaskTest.java", "javatests/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListenerTest.java", - "javatests/src/org/chromium/chrome/browser/gsa/GSAServiceClientTest.java", "javatests/src/org/chromium/chrome/browser/hardware_acceleration/ChromeTabbedActivityHWATest.java", "javatests/src/org/chromium/chrome/browser/hardware_acceleration/CustomTabActivityHWATest.java", "javatests/src/org/chromium/chrome/browser/hardware_acceleration/ManifestHWATest.java",
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/gsa/GSAServiceClientTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/gsa/GSAServiceClientTest.java deleted file mode 100644 index b7b99129..0000000 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/gsa/GSAServiceClientTest.java +++ /dev/null
@@ -1,75 +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. - -package org.chromium.chrome.browser.gsa; - -import android.content.ComponentName; -import android.content.Context; -import android.content.Intent; -import android.content.ServiceConnection; -import android.os.IBinder; -import android.support.test.InstrumentationRegistry; -import android.support.test.filters.SmallTest; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; - -import org.chromium.base.Log; -import org.chromium.chrome.test.ChromeJUnit4ClassRunner; - -import java.util.concurrent.Semaphore; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; - -/** Tests for GSAServiceClient. */ -@RunWith(ChromeJUnit4ClassRunner.class) -public class GSAServiceClientTest { - private static final String TAG = "GSAServiceClientTest"; - - @Test - @SmallTest - public void testGetPssForService() throws Exception { - Context context = InstrumentationRegistry.getInstrumentation().getTargetContext(); - - if (!GSAState.getInstance(context).isGsaAvailable()) { - Log.w(TAG, "GSA is not available"); - return; - } - - final AtomicReference<ComponentName> componentNameRef = new AtomicReference<>(); - final Semaphore semaphore = new Semaphore(0); - Intent intent = - new Intent(GSAServiceClient.GSA_SERVICE).setPackage(GSAState.SEARCH_INTENT_PACKAGE); - - ServiceConnection connection = new ServiceConnection() { - @Override - public void onServiceConnected(ComponentName name, IBinder service) { - componentNameRef.set(name); - semaphore.release(); - } - - @Override - public void onServiceDisconnected(ComponentName name) {} - }; - context.bindService(intent, connection, Context.BIND_AUTO_CREATE); - try { - Assert.assertTrue("Timeout", semaphore.tryAcquire(10, TimeUnit.SECONDS)); - - int pss = GSAServiceClient.getPssForService(componentNameRef.get()); - Assert.assertTrue(pss != GSAServiceClient.INVALID_PSS); - Assert.assertTrue(pss > 0); - } finally { - context.unbindService(connection); - } - } - - @Test - @SmallTest - public void testGetPssForServiceServiceNotFound() throws Exception { - ComponentName componentName = new ComponentName("unknown.package.name", "UnknownClass"); - int pss = GSAServiceClient.getPssForService(componentName); - Assert.assertTrue(pss == GSAServiceClient.INVALID_PSS); - } -}
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn index 20a1513..a97a8ba 100644 --- a/chrome/browser/BUILD.gn +++ b/chrome/browser/BUILD.gn
@@ -881,8 +881,6 @@ "page_load_metrics/user_input_tracker.h", "password_manager/chrome_password_manager_client.cc", "password_manager/chrome_password_manager_client.h", - "password_manager/password_manager_setting_migrator_service_factory.cc", - "password_manager/password_manager_setting_migrator_service_factory.h", "password_manager/password_manager_util_mac.h", "password_manager/password_manager_util_mac.mm", "password_manager/password_manager_util_win.cc",
diff --git a/chrome/browser/extensions/api/preference/preference_api.cc b/chrome/browser/extensions/api/preference/preference_api.cc index 7e6f635..3895a93 100644 --- a/chrome/browser/extensions/api/preference/preference_api.cc +++ b/chrome/browser/extensions/api/preference/preference_api.cc
@@ -36,6 +36,7 @@ #include "components/spellcheck/browser/pref_names.h" #include "components/translate/core/browser/translate_pref_names.h" #include "content/public/browser/notification_details.h" +#include "content/public/browser/notification_service.h" #include "content/public/browser/notification_source.h" #include "extensions/browser/extension_pref_value_map.h" #include "extensions/browser/extension_pref_value_map_factory.h" @@ -360,17 +361,17 @@ PreferenceEventRouter::PreferenceEventRouter(Profile* profile) : profile_(profile) { registrar_.Init(profile_->GetPrefs()); - incognito_registrar_.Init(profile_->GetOffTheRecordPrefs()); for (size_t i = 0; i < arraysize(kPrefMapping); ++i) { registrar_.Add(kPrefMapping[i].browser_pref, base::Bind(&PreferenceEventRouter::OnPrefChanged, base::Unretained(this), registrar_.prefs())); - incognito_registrar_.Add(kPrefMapping[i].browser_pref, - base::Bind(&PreferenceEventRouter::OnPrefChanged, - base::Unretained(this), - incognito_registrar_.prefs())); } + notification_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_CREATED, + content::NotificationService::AllSources()); + notification_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED, + content::NotificationService::AllSources()); + OnIncognitoProfileCreated(profile->GetOffTheRecordPrefs()); } PreferenceEventRouter::~PreferenceEventRouter() { } @@ -424,6 +425,43 @@ browser_pref); } +void PreferenceEventRouter::Observe( + int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) { + switch (type) { + case chrome::NOTIFICATION_PROFILE_CREATED: { + Profile* profile = content::Source<Profile>(source).ptr(); + if (profile != profile_ && profile->GetOriginalProfile() == profile_) { + OnIncognitoProfileCreated(profile->GetPrefs()); + } + break; + } + case chrome::NOTIFICATION_PROFILE_DESTROYED: { + Profile* profile = content::Source<Profile>(source).ptr(); + if (profile != profile_ && profile->GetOriginalProfile() == profile_) { + // The real PrefService is about to be destroyed so we must make sure we + // get the "dummy" one. + OnIncognitoProfileCreated(profile_->GetReadOnlyOffTheRecordPrefs()); + } + break; + } + default: + NOTREACHED(); + } +} + +void PreferenceEventRouter::OnIncognitoProfileCreated(PrefService* prefs) { + incognito_registrar_.reset(new PrefChangeRegistrar()); + incognito_registrar_->Init(prefs); + for (size_t i = 0; i < arraysize(kPrefMapping); ++i) { + incognito_registrar_->Add( + kPrefMapping[i].browser_pref, + base::Bind(&PreferenceEventRouter::OnPrefChanged, + base::Unretained(this), incognito_registrar_->prefs())); + } +} + void PreferenceAPIBase::SetExtensionControlledPref( const std::string& extension_id, const std::string& pref_key, @@ -619,7 +657,7 @@ return RespondNow(Error(keys::kPermissionErrorMessage, pref_key)); Profile* profile = Profile::FromBrowserContext(browser_context()); - PrefService* prefs = + const PrefService* prefs = incognito ? profile->GetOffTheRecordPrefs() : profile->GetPrefs(); const PrefService::Preference* pref = prefs->FindPreference(browser_pref); CHECK(pref);
diff --git a/chrome/browser/extensions/api/preference/preference_api.h b/chrome/browser/extensions/api/preference/preference_api.h index ff72eba7..78ba18d2 100644 --- a/chrome/browser/extensions/api/preference/preference_api.h +++ b/chrome/browser/extensions/api/preference/preference_api.h
@@ -14,6 +14,7 @@ #include "chrome/browser/extensions/chrome_extension_function.h" #include "components/prefs/pref_change_registrar.h" #include "content/public/browser/notification_observer.h" +#include "content/public/browser/notification_registrar.h" #include "extensions/browser/browser_context_keyed_api_factory.h" #include "extensions/browser/event_router.h" #include "extensions/browser/extension_prefs_scope.h" @@ -28,17 +29,25 @@ namespace extensions { class ExtensionPrefs; -class PreferenceEventRouter { +class PreferenceEventRouter : public content::NotificationObserver { public: explicit PreferenceEventRouter(Profile* profile); - virtual ~PreferenceEventRouter(); + ~PreferenceEventRouter() override; private: void OnPrefChanged(PrefService* pref_service, const std::string& pref_key); + // content::NotificationObserver: + void Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) override; + + void OnIncognitoProfileCreated(PrefService* prefs); + + content::NotificationRegistrar notification_registrar_; PrefChangeRegistrar registrar_; - PrefChangeRegistrar incognito_registrar_; + std::unique_ptr<PrefChangeRegistrar> incognito_registrar_; // Weak, owns us (transitively via ExtensionService). Profile* profile_;
diff --git a/chrome/browser/extensions/test_extension_prefs.cc b/chrome/browser/extensions/test_extension_prefs.cc index 871b1b7f..2e1740f8 100644 --- a/chrome/browser/extensions/test_extension_prefs.cc +++ b/chrome/browser/extensions/test_extension_prefs.cc
@@ -198,7 +198,8 @@ PrefService* TestExtensionPrefs::CreateIncognitoPrefService() const { return CreateIncognitoPrefServiceSyncable( pref_service_.get(), - new ExtensionPrefStore(extension_pref_value_map_.get(), true)); + new ExtensionPrefStore(extension_pref_value_map_.get(), true), + std::set<PrefValueStore::PrefStoreType>(), nullptr, nullptr); } void TestExtensionPrefs::set_extensions_disabled(bool extensions_disabled) {
diff --git a/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc b/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc index a3ea43e..9412c6e4 100644 --- a/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc +++ b/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
@@ -768,7 +768,7 @@ ProfileIOData* io_data = ProfileIOData::FromResourceContext(resource_context); signin::ProcessAccountConsistencyResponseHeaders( - request, GURL(), io_data, info->GetChildID(), info->GetRouteID()); + request, GURL(), io_data, info->GetWebContentsGetterForRequest()); // Built-in additional protection for the chrome web store origin. #if BUILDFLAG(ENABLE_EXTENSIONS) @@ -814,7 +814,7 @@ signin::FixAccountConsistencyRequestHeader( request, redirect_url, io_data, info->GetChildID(), info->GetRouteID()); signin::ProcessAccountConsistencyResponseHeaders( - request, redirect_url, io_data, info->GetChildID(), info->GetRouteID()); + request, redirect_url, io_data, info->GetWebContentsGetterForRequest()); if (io_data->loading_predictor_observer()) { io_data->loading_predictor_observer()->OnRequestRedirected(
diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc index 0c9d90c2..04445195 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client.cc
@@ -43,7 +43,6 @@ #include "components/password_manager/core/browser/password_form_manager.h" #include "components/password_manager/core/browser/password_manager_internals_service.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h" -#include "components/password_manager/core/browser/password_manager_settings_migration_experiment.h" #include "components/password_manager/core/browser/password_manager_util.h" #include "components/password_manager/core/common/credential_manager_types.h" #include "components/password_manager/core/common/password_manager_features.h"
diff --git a/chrome/browser/password_manager/password_manager_setting_migrator_service_factory.cc b/chrome/browser/password_manager/password_manager_setting_migrator_service_factory.cc deleted file mode 100644 index ae8786a5..0000000 --- a/chrome/browser/password_manager/password_manager_setting_migrator_service_factory.cc +++ /dev/null
@@ -1,49 +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. - -#include "chrome/browser/password_manager/password_manager_setting_migrator_service_factory.h" - -#include "chrome/browser/prefs/pref_service_syncable_util.h" -#include "chrome/browser/profiles/incognito_helpers.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/sync/profile_sync_service_factory.h" -#include "components/keyed_service/content/browser_context_dependency_manager.h" -#include "components/password_manager/sync/browser/password_manager_setting_migrator_service.h" - -// static -PasswordManagerSettingMigratorServiceFactory* -PasswordManagerSettingMigratorServiceFactory::GetInstance() { - return base::Singleton<PasswordManagerSettingMigratorServiceFactory>::get(); -} - -// static -password_manager::PasswordManagerSettingMigratorService* -PasswordManagerSettingMigratorServiceFactory::GetForProfile(Profile* profile) { - return static_cast<password_manager::PasswordManagerSettingMigratorService*>( - GetInstance()->GetServiceForBrowserContext(profile, true)); -} - -PasswordManagerSettingMigratorServiceFactory:: - PasswordManagerSettingMigratorServiceFactory() - : BrowserContextKeyedServiceFactory( - "PasswordManagerSettingMigratorService", - BrowserContextDependencyManager::GetInstance()) { - DependsOn(ProfileSyncServiceFactory::GetInstance()); -} - -PasswordManagerSettingMigratorServiceFactory:: - ~PasswordManagerSettingMigratorServiceFactory() {} - -KeyedService* -PasswordManagerSettingMigratorServiceFactory::BuildServiceInstanceFor( - content::BrowserContext* context) const { - Profile* profile = static_cast<Profile*>(context); - return new password_manager::PasswordManagerSettingMigratorService( - PrefServiceSyncableFromProfile(profile)); -} - -bool PasswordManagerSettingMigratorServiceFactory:: - ServiceIsCreatedWithBrowserContext() const { - return true; -}
diff --git a/chrome/browser/password_manager/password_manager_setting_migrator_service_factory.h b/chrome/browser/password_manager/password_manager_setting_migrator_service_factory.h deleted file mode 100644 index 24c5464..0000000 --- a/chrome/browser/password_manager/password_manager_setting_migrator_service_factory.h +++ /dev/null
@@ -1,37 +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 CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_SETTING_MIGRATOR_SERVICE_FACTORY_H_ -#define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_SETTING_MIGRATOR_SERVICE_FACTORY_H_ - -#include "base/memory/singleton.h" -#include "components/keyed_service/content/browser_context_keyed_service_factory.h" - -class Profile; - -namespace password_manager { -class PasswordManagerSettingMigratorService; -} - -class PasswordManagerSettingMigratorServiceFactory - : public BrowserContextKeyedServiceFactory { - public: - static PasswordManagerSettingMigratorServiceFactory* GetInstance(); - static password_manager::PasswordManagerSettingMigratorService* GetForProfile( - Profile* profile); - - private: - friend struct base::DefaultSingletonTraits< - PasswordManagerSettingMigratorServiceFactory>; - - PasswordManagerSettingMigratorServiceFactory(); - ~PasswordManagerSettingMigratorServiceFactory() override; - - // BrowserContextKeyedServiceFactory - KeyedService* BuildServiceInstanceFor( - content::BrowserContext* profile) const override; - bool ServiceIsCreatedWithBrowserContext() const override; -}; - -#endif // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_SETTING_MIGRATOR_SERVICE_FACTORY_H_
diff --git a/chrome/browser/prefs/pref_service_syncable_util.cc b/chrome/browser/prefs/pref_service_syncable_util.cc index 68c2f27..8a552a5 100644 --- a/chrome/browser/prefs/pref_service_syncable_util.cc +++ b/chrome/browser/prefs/pref_service_syncable_util.cc
@@ -30,7 +30,10 @@ sync_preferences::PrefServiceSyncable* CreateIncognitoPrefServiceSyncable( sync_preferences::PrefServiceSyncable* pref_service, - PrefStore* incognito_extension_pref_store) { + PrefStore* incognito_extension_pref_store, + std::set<PrefValueStore::PrefStoreType> already_connected_types, + service_manager::Connector* incognito_connector, + service_manager::Connector* user_connector) { // List of keys that cannot be changed in the user prefs file by the incognito // profile. All preferences that store information about the browsing history // or behavior of the user should have this property. @@ -42,5 +45,6 @@ overlay_pref_names.push_back(proxy_config::prefs::kProxy); #endif return pref_service->CreateIncognitoPrefService( - incognito_extension_pref_store, overlay_pref_names); + incognito_extension_pref_store, overlay_pref_names, + std::move(already_connected_types), incognito_connector, user_connector); }
diff --git a/chrome/browser/prefs/pref_service_syncable_util.h b/chrome/browser/prefs/pref_service_syncable_util.h index d4149d9..1bcdc321 100644 --- a/chrome/browser/prefs/pref_service_syncable_util.h +++ b/chrome/browser/prefs/pref_service_syncable_util.h
@@ -5,9 +5,17 @@ #ifndef CHROME_BROWSER_PREFS_PREF_SERVICE_SYNCABLE_UTIL_H_ #define CHROME_BROWSER_PREFS_PREF_SERVICE_SYNCABLE_UTIL_H_ +#include <set> + +#include "components/prefs/pref_value_store.h" + class PrefStore; class Profile; +namespace service_manager { +class Connector; +} + namespace sync_preferences { class PrefServiceSyncable; } @@ -30,8 +38,16 @@ // a fresh non-persistent overlay for the user pref store and an individual // extension pref store (to cache the effective extension prefs for incognito // windows). +// +// If the Mojo pref service is in use |incognito_connector| and |user_connector| +// must be non-null and |already_connected_types| should be the set of +// |PrefStore|s that are running in the current service and thus don't need to +// be connected to. sync_preferences::PrefServiceSyncable* CreateIncognitoPrefServiceSyncable( sync_preferences::PrefServiceSyncable* pref_service, - PrefStore* incognito_extension_pref_store); + PrefStore* incognito_extension_pref_store, + std::set<PrefValueStore::PrefStoreType> already_connected_types, + service_manager::Connector* incognito_connector, + service_manager::Connector* user_connector); #endif // CHROME_BROWSER_PREFS_PREF_SERVICE_SYNCABLE_UTIL_H_
diff --git a/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc b/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc index aecdbeb..a3f69e7 100644 --- a/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc +++ b/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
@@ -36,7 +36,6 @@ #include "chrome/browser/notifications/extension_welcome_notification_factory.h" #include "chrome/browser/notifications/notifier_state_tracker_factory.h" #include "chrome/browser/ntp_snippets/content_suggestions_service_factory.h" -#include "chrome/browser/password_manager/password_manager_setting_migrator_service_factory.h" #include "chrome/browser/password_manager/password_store_factory.h" #include "chrome/browser/plugins/plugin_prefs_factory.h" #include "chrome/browser/policy/cloud/policy_header_service_factory.h" @@ -285,7 +284,6 @@ #endif ContentSuggestionsServiceFactory::GetInstance(); PasswordStoreFactory::GetInstance(); - PasswordManagerSettingMigratorServiceFactory::GetInstance(); #if !defined(OS_ANDROID) PinnedTabServiceFactory::GetInstance(); ThemeServiceFactory::GetInstance();
diff --git a/chrome/browser/profiles/off_the_record_profile_impl.cc b/chrome/browser/profiles/off_the_record_profile_impl.cc index ca7bf1f..33eb014 100644 --- a/chrome/browser/profiles/off_the_record_profile_impl.cc +++ b/chrome/browser/profiles/off_the_record_profile_impl.cc
@@ -14,6 +14,7 @@ #include "base/memory/ptr_util.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" +#include "base/threading/sequenced_worker_pool.h" #include "build/build_config.h" #include "chrome/browser/background/background_contents_service_factory.h" #include "chrome/browser/background_sync/background_sync_controller_factory.h" @@ -33,6 +34,7 @@ #include "chrome/browser/permissions/permission_manager_factory.h" #include "chrome/browser/plugins/chrome_plugin_service_filter.h" #include "chrome/browser/plugins/plugin_prefs.h" +#include "chrome/browser/prefs/browser_prefs.h" #include "chrome/browser/prefs/incognito_mode_prefs.h" #include "chrome/browser/prefs/pref_service_syncable_util.h" #include "chrome/browser/profiles/profile_manager.h" @@ -41,6 +43,7 @@ #include "chrome/browser/themes/theme_service.h" #include "chrome/browser/ui/webui/extensions/extension_icon_source.h" #include "chrome/common/chrome_constants.h" +#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/features.h" @@ -59,6 +62,9 @@ #include "net/http/http_server_properties.h" #include "net/http/transport_security_state.h" #include "ppapi/features/features.h" +#include "services/preferences/public/cpp/pref_service_main.h" +#include "services/preferences/public/interfaces/preferences.mojom.h" +#include "services/service_manager/public/cpp/service.h" #include "storage/browser/database/database_tracker.h" #if defined(OS_ANDROID) @@ -84,6 +90,8 @@ #include "chrome/browser/extensions/extension_special_storage_policy.h" #include "components/guest_view/browser/guest_view_manager.h" #include "extensions/browser/api/web_request/web_request_api.h" +#include "extensions/browser/extension_pref_store.h" +#include "extensions/browser/extension_pref_value_map_factory.h" #include "extensions/browser/extension_system.h" #include "extensions/common/extension.h" #endif @@ -118,13 +126,37 @@ } // namespace #endif +PrefStore* CreateExtensionPrefStore(Profile* profile, + bool incognito_pref_store) { +#if BUILDFLAG(ENABLE_EXTENSIONS) + return new ExtensionPrefStore( + ExtensionPrefValueMapFactory::GetForBrowserContext(profile), + incognito_pref_store); +#else + return NULL; +#endif +} + OffTheRecordProfileImpl::OffTheRecordProfileImpl(Profile* real_profile) : profile_(real_profile), - prefs_(PrefServiceSyncableIncognitoFromProfile(real_profile)), start_time_(Time::Now()) { + // Must happen before we ask for prefs as prefs needs the connection to the + // service manager, which is set up in Initialize. BrowserContext::Initialize(this, profile_->GetPath()); + service_manager::Connector* otr_connector = nullptr; + service_manager::Connector* user_connector = nullptr; + std::set<PrefValueStore::PrefStoreType> already_connected_types; + if (features::PrefServiceEnabled()) { + otr_connector = content::BrowserContext::GetConnectorFor(this); + user_connector = content::BrowserContext::GetConnectorFor(profile_); + already_connected_types = chrome::InProcessPrefStores(); + } + prefs_.reset(CreateIncognitoPrefServiceSyncable( + PrefServiceSyncableFromProfile(profile_), + CreateExtensionPrefStore(profile_, true), + std::move(already_connected_types), otr_connector, user_connector)); // Register on BrowserContext. - user_prefs::UserPrefs::Set(this, prefs_); + user_prefs::UserPrefs::Set(this, prefs_.get()); } void OffTheRecordProfileImpl::Init() { @@ -310,15 +342,15 @@ } PrefService* OffTheRecordProfileImpl::GetPrefs() { - return prefs_; + return prefs_.get(); } const PrefService* OffTheRecordProfileImpl::GetPrefs() const { - return prefs_; + return prefs_.get(); } PrefService* OffTheRecordProfileImpl::GetOffTheRecordPrefs() { - return prefs_; + return prefs_.get(); } DownloadManagerDelegate* OffTheRecordProfileImpl::GetDownloadManagerDelegate() { @@ -348,6 +380,19 @@ .get(); } +void OffTheRecordProfileImpl::RegisterInProcessServices( + StaticServiceMap* services) { + if (features::PrefServiceEnabled()) { + content::ServiceInfo info; + info.factory = base::Bind( + &prefs::CreatePrefService, chrome::ExpectedPrefStores(), + make_scoped_refptr(content::BrowserThread::GetBlockingPool())); + info.task_runner = content::BrowserThread::GetTaskRunnerForThread( + content::BrowserThread::IO); + services->insert(std::make_pair(prefs::mojom::kServiceName, info)); + } +} + net::URLRequestContextGetter* OffTheRecordProfileImpl::GetRequestContext() { return GetDefaultStoragePartition(this)->GetURLRequestContext(); }
diff --git a/chrome/browser/profiles/off_the_record_profile_impl.h b/chrome/browser/profiles/off_the_record_profile_impl.h index 46df7f1..6574d6c 100644 --- a/chrome/browser/profiles/off_the_record_profile_impl.h +++ b/chrome/browser/profiles/off_the_record_profile_impl.h
@@ -71,6 +71,7 @@ net::URLRequestContextGetter* CreateMediaRequestContextForStoragePartition( const base::FilePath& partition_path, bool in_memory) override; + void RegisterInProcessServices(StaticServiceMap* services) override; net::SSLConfigService* GetSSLConfigService() override; bool IsSameProfile(Profile* profile) override; Time GetStartTime() const override; @@ -137,8 +138,7 @@ // The real underlying profile. Profile* profile_; - // Weak pointer owned by |profile_|. - sync_preferences::PrefServiceSyncable* prefs_; + std::unique_ptr<sync_preferences::PrefServiceSyncable> prefs_; #if !defined(OS_ANDROID) std::unique_ptr<content::HostZoomMap::Subscription> track_zoom_subscription_;
diff --git a/chrome/browser/profiles/profile.cc b/chrome/browser/profiles/profile.cc index c56f39e..5847e19 100644 --- a/chrome/browser/profiles/profile.cc +++ b/chrome/browser/profiles/profile.cc
@@ -115,6 +115,10 @@ } #endif // !defined(OS_ANDROID) +PrefService* Profile::GetReadOnlyOffTheRecordPrefs() { + return GetOffTheRecordPrefs(); +} + Profile::Delegate::~Delegate() { }
diff --git a/chrome/browser/profiles/profile.h b/chrome/browser/profiles/profile.h index 39cf710..dbbfa07e 100644 --- a/chrome/browser/profiles/profile.h +++ b/chrome/browser/profiles/profile.h
@@ -191,6 +191,11 @@ // time that this method is called. virtual PrefService* GetOffTheRecordPrefs() = 0; + // Like GetOffTheRecordPrefs but gives a read-only view of prefs that can be + // used even if there's no OTR profile at the moment + // (i.e. HasOffTheRecordProfile is false). + virtual PrefService* GetReadOnlyOffTheRecordPrefs(); + // Returns the main request context. virtual net::URLRequestContextGetter* GetRequestContext() = 0;
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc index a345048d..e885f56 100644 --- a/chrome/browser/profiles/profile_impl.cc +++ b/chrome/browser/profiles/profile_impl.cc
@@ -834,7 +834,6 @@ void ProfileImpl::DestroyOffTheRecordProfile() { off_the_record_profile_.reset(); - otr_prefs_->ClearMutableValues(); #if BUILDFLAG(ENABLE_EXTENSIONS) ExtensionPrefValueMapFactory::GetForBrowserContext(this)-> ClearAllIncognitoSessionOnlyPreferences(); @@ -998,14 +997,27 @@ #endif // !defined(OS_ANDROID) PrefService* ProfileImpl::GetOffTheRecordPrefs() { - DCHECK(prefs_); - if (!otr_prefs_) { - // The new ExtensionPrefStore is ref_counted and the new PrefService - // stores a reference so that we do not leak memory here. - otr_prefs_.reset(CreateIncognitoPrefServiceSyncable( - prefs_.get(), CreateExtensionPrefStore(this, true))); + if (HasOffTheRecordProfile()) { + return GetOffTheRecordProfile()->GetPrefs(); + } else { + // The extensions preference API and many tests call this method even when + // there's no OTR profile, in order to figure out what a pref value would + // have been returned if an OTR profile existed. To support that case we + // return a dummy PrefService here. + // + // TODO(crbug.com/734484): Don't call this method when there's no OTR + // profile (and return null for such calls). + return GetReadOnlyOffTheRecordPrefs(); } - return otr_prefs_.get(); +} + +PrefService* ProfileImpl::GetReadOnlyOffTheRecordPrefs() { + if (!dummy_otr_prefs_) { + dummy_otr_prefs_.reset(CreateIncognitoPrefServiceSyncable( + prefs_.get(), CreateExtensionPrefStore(this, true), + std::set<PrefValueStore::PrefStoreType>(), nullptr, nullptr)); + } + return dummy_otr_prefs_.get(); } content::ResourceContext* ProfileImpl::GetResourceContext() {
diff --git a/chrome/browser/profiles/profile_impl.h b/chrome/browser/profiles/profile_impl.h index 24c41e5..0b5e691 100644 --- a/chrome/browser/profiles/profile_impl.h +++ b/chrome/browser/profiles/profile_impl.h
@@ -128,6 +128,7 @@ ChromeZoomLevelPrefs* GetZoomLevelPrefs() override; #endif PrefService* GetOffTheRecordPrefs() override; + PrefService* GetReadOnlyOffTheRecordPrefs() override; net::URLRequestContextGetter* GetRequestContext() override; net::URLRequestContextGetter* GetRequestContextForExtensions() override; net::SSLConfigService* GetSSLConfigService() override; @@ -232,7 +233,9 @@ // first. scoped_refptr<user_prefs::PrefRegistrySyncable> pref_registry_; std::unique_ptr<sync_preferences::PrefServiceSyncable> prefs_; - std::unique_ptr<sync_preferences::PrefServiceSyncable> otr_prefs_; + // See comment in GetOffTheRecordPrefs. Field exists so something owns the + // dummy. + std::unique_ptr<sync_preferences::PrefServiceSyncable> dummy_otr_prefs_; ProfileImplIOData::Handle io_data_; #if BUILDFLAG(ENABLE_EXTENSIONS) scoped_refptr<ExtensionSpecialStoragePolicy>
diff --git a/chrome/browser/profiles/profile_manager.cc b/chrome/browser/profiles/profile_manager.cc index fae8756..b40ced7d 100644 --- a/chrome/browser/profiles/profile_manager.cc +++ b/chrome/browser/profiles/profile_manager.cc
@@ -35,7 +35,6 @@ #include "chrome/browser/invalidation/profile_invalidation_provider_factory.h" #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h" #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h" -#include "chrome/browser/password_manager/password_manager_setting_migrator_service_factory.h" #include "chrome/browser/password_manager/password_store_factory.h" #include "chrome/browser/prefs/incognito_mode_prefs.h" #include "chrome/browser/profiles/bookmark_model_loaded_observer.h" @@ -75,7 +74,6 @@ #include "components/invalidation/impl/profile_invalidation_provider.h" #include "components/invalidation/public/invalidation_service.h" #include "components/password_manager/core/browser/password_store.h" -#include "components/password_manager/sync/browser/password_manager_setting_migrator_service.h" #include "components/prefs/pref_service.h" #include "components/prefs/scoped_user_pref_update.h" #include "components/search_engines/default_search_manager.h" @@ -1263,14 +1261,6 @@ ->SetupInvalidationsOnProfileLoad(invalidation_service); AccountReconcilorFactory::GetForProfile(profile); - // Service is responsible for migration of the legacy password manager - // preference which controls behaviour of the Chrome to the new preference - // which controls password management behaviour on Chrome and Android. After - // migration will be performed for all users it's planned to remove the - // migration code, rough time estimates are Q1 2016. - PasswordManagerSettingMigratorServiceFactory::GetForProfile(profile) - ->InitializeMigration(ProfileSyncServiceFactory::GetForProfile(profile)); - #if defined(OS_ANDROID) // TODO(b/678590): create services during profile startup. // Service is responsible for fetching content snippets for the NTP.
diff --git a/chrome/browser/signin/chrome_signin_helper.cc b/chrome/browser/signin/chrome_signin_helper.cc index 8348c8a..a0b599c 100644 --- a/chrome/browser/signin/chrome_signin_helper.cc +++ b/chrome/browser/signin/chrome_signin_helper.cc
@@ -19,7 +19,6 @@ #include "components/signin/core/browser/signin_header_helper.h" #include "components/signin/core/common/profile_management_switches.h" #include "content/public/browser/browser_thread.h" -#include "content/public/browser/resource_request_info.h" #include "content/public/browser/web_contents.h" #include "google_apis/gaia/gaia_auth_util.h" #include "net/http/http_response_headers.h" @@ -46,9 +45,10 @@ // Processes the mirror response header on the UI thread. Currently depending // on the value of |header_value|, it either shows the profile avatar menu, or // opens an incognito window/tab. -void ProcessMirrorHeaderUIThread(int child_id, - int route_id, - ManageAccountsParams manage_accounts_params) { +void ProcessMirrorHeaderUIThread( + ManageAccountsParams manage_accounts_params, + const content::ResourceRequestInfo::WebContentsGetter& + web_contents_getter) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_EQ(switches::AccountConsistencyMethod::kMirror, switches::GetAccountConsistencyMethod()); @@ -56,8 +56,7 @@ GAIAServiceType service_type = manage_accounts_params.service_type; DCHECK_NE(GAIA_SERVICE_TYPE_NONE, service_type); - content::WebContents* web_contents = - tab_util::GetWebContentsByID(child_id, route_id); + content::WebContents* web_contents = web_contents_getter.Run(); if (!web_contents) return; @@ -109,10 +108,11 @@ // Looks for the X-Chrome-Manage-Accounts response header, and if found, // tries to show the avatar bubble in the browser identified by the // child/route id. Must be called on IO thread. -void ProcessMirrorResponseHeaderIfExists(net::URLRequest* request, - ProfileIOData* io_data, - int child_id, - int route_id) { +void ProcessMirrorResponseHeaderIfExists( + net::URLRequest* request, + ProfileIOData* io_data, + const content::ResourceRequestInfo::WebContentsGetter& + web_contents_getter) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); const content::ResourceRequestInfo* info = @@ -152,11 +152,9 @@ if (params.service_type == GAIA_SERVICE_TYPE_NONE) return; - params.child_id = child_id; - params.route_id = route_id; content::BrowserThread::PostTask( content::BrowserThread::UI, FROM_HERE, - base::Bind(ProcessMirrorHeaderUIThread, child_id, route_id, params)); + base::Bind(ProcessMirrorHeaderUIThread, params, web_contents_getter)); } #if !defined(OS_ANDROID) @@ -242,19 +240,20 @@ io_data->GetCookieSettings(), profile_mode_mask); } -void ProcessAccountConsistencyResponseHeaders(net::URLRequest* request, - const GURL& redirect_url, - ProfileIOData* io_data, - int child_id, - int route_id) { +void ProcessAccountConsistencyResponseHeaders( + net::URLRequest* request, + const GURL& redirect_url, + ProfileIOData* io_data, + const content::ResourceRequestInfo::WebContentsGetter& + web_contents_getter) { if (redirect_url.is_empty()) { // This is not a redirect. // See if the response contains the X-Chrome-Manage-Accounts header. If so // show the profile avatar bubble so that user can complete signin/out // action the native UI. - signin::ProcessMirrorResponseHeaderIfExists(request, io_data, child_id, - route_id); + signin::ProcessMirrorResponseHeaderIfExists(request, io_data, + web_contents_getter); } else { // This is a redirect.
diff --git a/chrome/browser/signin/chrome_signin_helper.h b/chrome/browser/signin/chrome_signin_helper.h index 15b52b90..1c73f0e 100644 --- a/chrome/browser/signin/chrome_signin_helper.h +++ b/chrome/browser/signin/chrome_signin_helper.h
@@ -7,6 +7,8 @@ #include <string> +#include "content/public/browser/resource_request_info.h" + namespace net { class URLRequest; } @@ -33,11 +35,11 @@ // Processes account consistency response headers (X-Chrome-Manage-Accounts and // Dice). |redirect_url| is empty if the request is not a redirect. -void ProcessAccountConsistencyResponseHeaders(net::URLRequest* request, - const GURL& redirect_url, - ProfileIOData* io_data, - int child_id, - int route_id); +void ProcessAccountConsistencyResponseHeaders( + net::URLRequest* request, + const GURL& redirect_url, + ProfileIOData* io_data, + const content::ResourceRequestInfo::WebContentsGetter& web_contents_getter); }; // namespace signin
diff --git a/chrome/browser/sync/test/integration/password_manager_setting_migrator_helper.cc b/chrome/browser/sync/test/integration/password_manager_setting_migrator_helper.cc deleted file mode 100644 index 3fa4a67..0000000 --- a/chrome/browser/sync/test/integration/password_manager_setting_migrator_helper.cc +++ /dev/null
@@ -1,59 +0,0 @@ -// Copyright (c) 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. - -#include "chrome/browser/sync/test/integration/password_manager_setting_migrator_helper.h" - -#include "base/metrics/field_trial.h" -#include "chrome/browser/chrome_notification_types.h" -#include "chrome/browser/password_manager/password_manager_setting_migrator_service_factory.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/sync/profile_sync_service_factory.h" -#include "chrome/browser/sync/test/integration/preferences_helper.h" -#include "components/browser_sync/profile_sync_service.h" -#include "components/password_manager/core/browser/password_manager_settings_migration_experiment.h" -#include "components/password_manager/core/common/password_manager_pref_names.h" -#include "components/password_manager/sync/browser/password_manager_setting_migrator_service.h" -#include "components/prefs/pref_service.h" -#include "content/public/browser/notification_details.h" -#include "content/public/browser/notification_service.h" -#include "content/public/browser/notification_source.h" -#include "testing/gtest/include/gtest/gtest.h" - -using preferences_helper::GetPrefs; - -namespace { - -const char kFieldTrialName[] = "PasswordManagerSettingsMigration"; -const char kEnabledGroupName[] = "Enable"; - -} // namespace - -namespace password_manager_setting_migrater_helper { - -bool EnsureFieldTrialSetup() { - if (base::FieldTrialList::TrialExists(kFieldTrialName)) { - DCHECK(password_manager::IsSettingsMigrationActive()); - return false; - } - base::FieldTrialList::CreateFieldTrial(kFieldTrialName, kEnabledGroupName); - return true; -} - -void InitializePreferencesMigration(Profile* profile) { - PasswordManagerSettingMigratorServiceFactory::GetForProfile(profile) - ->InitializeMigration(ProfileSyncServiceFactory::GetForProfile(profile)); -} - -void ExpectPrefValuesOnClient(int index, - bool new_pref_value, - bool old_pref_value) { - using password_manager::prefs::kCredentialsEnableService; - using password_manager::prefs::kPasswordManagerSavingEnabled; - EXPECT_EQ(new_pref_value, - GetPrefs(index)->GetBoolean(kCredentialsEnableService)); - EXPECT_EQ(old_pref_value, - GetPrefs(index)->GetBoolean(kPasswordManagerSavingEnabled)); -} - -} // namespace password_manager_setting_migrater_helper
diff --git a/chrome/browser/sync/test/integration/single_client_password_manager_setting_migrator_service_sync_test.cc b/chrome/browser/sync/test/integration/single_client_password_manager_setting_migrator_service_sync_test.cc deleted file mode 100644 index 0fddb9a..0000000 --- a/chrome/browser/sync/test/integration/single_client_password_manager_setting_migrator_service_sync_test.cc +++ /dev/null
@@ -1,260 +0,0 @@ -// Copyright (c) 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. - -#include "base/json/json_writer.h" -#include "base/macros.h" -#include "chrome/browser/prefs/pref_service_syncable_util.h" -#include "chrome/browser/sync/test/integration/password_manager_setting_migrator_helper.h" -#include "chrome/browser/sync/test/integration/preferences_helper.h" -#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h" -#include "chrome/browser/sync/test/integration/sync_test.h" -#include "chrome/browser/sync/test/integration/updated_progress_marker_checker.h" -#include "chrome/common/pref_names.h" -#include "components/password_manager/core/common/password_manager_pref_names.h" -#include "components/password_manager/sync/browser/password_manager_setting_migrator_service.h" -#include "components/prefs/pref_service.h" -#include "components/sync/base/model_type.h" -#include "components/sync/protocol/preference_specifics.pb.h" -#include "components/sync/protocol/priority_preference_specifics.pb.h" -#include "components/sync/protocol/sync.pb.h" -#include "components/sync/test/fake_server/unique_client_entity.h" -#include "components/sync_preferences/pref_service_syncable.h" -#include "components/sync_preferences/pref_service_syncable_observer.h" -#include "content/public/browser/notification_details.h" -#include "content/public/browser/notification_service.h" -#include "content/public/browser/notification_source.h" -#include "content/public/test/test_browser_thread_bundle.h" - -using password_manager_setting_migrater_helper::ExpectPrefValuesOnClient; -using preferences_helper::GetPrefs; -using password_manager::prefs::kCredentialsEnableService; -using password_manager::prefs::kPasswordManagerSavingEnabled; - -namespace { - -void InjectPreferenceValueToFakeServer(fake_server::FakeServer* fake_server, - const std::string& name, - bool value) { - std::string serialized; - base::Value bool_value(value); - base::JSONWriter::Write(bool_value, &serialized); - sync_pb::EntitySpecifics specifics; - sync_pb::PreferenceSpecifics* pref = nullptr; - if (name == kPasswordManagerSavingEnabled) { - pref = specifics.mutable_preference(); - } else if (name == kCredentialsEnableService) { - pref = specifics.mutable_priority_preference()->mutable_preference(); - } else { - NOTREACHED() << "Wrong preference name: " << name; - } - pref->set_name(name); - pref->set_value(serialized); - const std::string id = "settings_reconciliation"; - fake_server->InjectEntity( - fake_server::UniqueClientEntity::CreateForInjection(id, specifics)); -} - -} // namespace - -class SingleClientPasswordManagerSettingMigratorServiceSyncTest - : public SyncTest { - public: - SingleClientPasswordManagerSettingMigratorServiceSyncTest() - : SyncTest(SINGLE_CLIENT) { - password_manager::PasswordManagerSettingMigratorService:: - set_force_disabled_for_testing(true); - } - - ~SingleClientPasswordManagerSettingMigratorServiceSyncTest() override {} - - void AssertPrefValues(bool new_pref_value, bool old_pref_value) { - ExpectPrefValuesOnClient(0, new_pref_value, old_pref_value); - } - - void SetLocalPrefValues(bool new_pref_local_value, - bool old_pref_local_value) { - PrefService* prefs = GetPrefs(0); - prefs->SetBoolean(kCredentialsEnableService, new_pref_local_value); - prefs->SetBoolean(kPasswordManagerSavingEnabled, old_pref_local_value); - AssertPrefValues(new_pref_local_value, old_pref_local_value); - } - - void InjectNewValues(bool new_pref_sync_value, bool old_pref_sync_value) { - InjectPreferenceValueToFakeServer( - GetFakeServer(), kCredentialsEnableService, new_pref_sync_value); - InjectPreferenceValueToFakeServer( - GetFakeServer(), kPasswordManagerSavingEnabled, old_pref_sync_value); - } - - void InitMigrationServiceAndSync() { - // Set up sync without prefs first to suppress the migration logic. - syncer::ModelTypeSet types = syncer::UserSelectableTypes(); - types.Remove(syncer::PREFERENCES); - ASSERT_TRUE(GetClient(0)->SetupSync(types)); - password_manager::PasswordManagerSettingMigratorService:: - set_force_disabled_for_testing(false); - password_manager_setting_migrater_helper::EnsureFieldTrialSetup(); - password_manager_setting_migrater_helper::InitializePreferencesMigration( - GetProfile(0)); - // Now enable prefs, the completion of which will trigger the migration - // logic. - ASSERT_TRUE(GetClient(0)->EnableSyncForAllDatatypes()); - } - - private: - DISALLOW_COPY_AND_ASSIGN( - SingleClientPasswordManagerSettingMigratorServiceSyncTest); -}; - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOnOnSyncOffOff) { - ASSERT_TRUE(SetupClients()); - SetLocalPrefValues(true /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); - InjectNewValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); -} - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOnOnSyncOnOff) { - ASSERT_TRUE(SetupClients()); - InjectNewValues(true /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); -} - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOnOnSyncOffOn) { - ASSERT_TRUE(SetupClients()); - SetLocalPrefValues(true /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); - InjectNewValues(false /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); -} - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOnOffSyncOnOn) { - ASSERT_TRUE(SetupClients()); - SetLocalPrefValues(true /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InjectNewValues(true /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(true /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); -} - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOnOffSyncOnOff) { - ASSERT_TRUE(SetupClients()); - SetLocalPrefValues(true /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InjectNewValues(true /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); -} - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOnOffSyncOffOn) { - ASSERT_TRUE(SetupClients()); - SetLocalPrefValues(true /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InjectNewValues(false /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); -} - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOffOffSyncOffOn) { - ASSERT_TRUE(SetupClients()); - SetLocalPrefValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InjectNewValues(false /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(true /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); -} - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOffOffSyncOnOff) { - ASSERT_TRUE(SetupClients()); - SetLocalPrefValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InjectNewValues(true /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(true /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); -} - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOffOnSyncOffOn) { - ASSERT_TRUE(SetupClients()); - SetLocalPrefValues(false /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); - InjectNewValues(false /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); -} - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOffOffSyncOffOff) { - ASSERT_TRUE(SetupClients()); - SetLocalPrefValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InjectNewValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(false /* kCredentialsEnableService */, - false /* kPasswordManagerSavingEnabled */); -} - -IN_PROC_BROWSER_TEST_F( - SingleClientPasswordManagerSettingMigratorServiceSyncTest, - LocalOnOnSyncOnOn) { - ASSERT_TRUE(SetupClients()); - SetLocalPrefValues(true /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); - InjectNewValues(true /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); - InitMigrationServiceAndSync(); - ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); - AssertPrefValues(true /* kCredentialsEnableService */, - true /* kPasswordManagerSavingEnabled */); -}
diff --git a/chrome/browser/sync/test/integration/two_client_password_manager_setting_migrator_service_sync_test.cc b/chrome/browser/sync/test/integration/two_client_password_manager_setting_migrator_service_sync_test.cc deleted file mode 100644 index c945621e..0000000 --- a/chrome/browser/sync/test/integration/two_client_password_manager_setting_migrator_service_sync_test.cc +++ /dev/null
@@ -1,121 +0,0 @@ -// Copyright (c) 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. - -#include "base/macros.h" -#include "chrome/browser/sync/test/integration/password_manager_setting_migrator_helper.h" -#include "chrome/browser/sync/test/integration/preferences_helper.h" -#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h" -#include "chrome/browser/sync/test/integration/sync_integration_test_util.h" -#include "chrome/browser/sync/test/integration/sync_test.h" -#include "chrome/common/pref_names.h" -#include "components/password_manager/core/common/password_manager_pref_names.h" -#include "components/prefs/pref_service.h" -#include "components/sync_preferences/pref_service_syncable.h" -#include "content/public/test/test_browser_thread_bundle.h" - -using password_manager_setting_migrater_helper::ExpectPrefValuesOnClient; -using preferences_helper::GetPrefs; -using password_manager::prefs::kPasswordManagerSavingEnabled; -using password_manager::prefs::kCredentialsEnableService; - -class TwoClientsPasswordManagerSettingMigratorServiceSyncTest - : public SyncTest { - public: - TwoClientsPasswordManagerSettingMigratorServiceSyncTest() - : SyncTest(TWO_CLIENT) {} - - ~TwoClientsPasswordManagerSettingMigratorServiceSyncTest() override {} - - bool SetupClients() override { - if (!SyncTest::SetupClients()) - return false; - DisableVerifier(); - return true; - } - - // Changes the |pref_name| preference value on the client with |index| and - // checks that the value is the same on both clients after the change. - void TestPrefChangeOnClient(int index, const char* pref_name) { - ASSERT_TRUE(BooleanPrefMatchChecker(kPasswordManagerSavingEnabled).Wait()); - ASSERT_TRUE(BooleanPrefMatchChecker(kCredentialsEnableService).Wait()); - - preferences_helper::ChangeBooleanPref(index, pref_name); - // Check that changed pref has the same value on both clients - ASSERT_TRUE(BooleanPrefMatchChecker(kPasswordManagerSavingEnabled).Wait()); - ASSERT_TRUE(BooleanPrefMatchChecker(kCredentialsEnableService).Wait()); - } - - void EnsureMigrationStartsForClient(int index) { - if (password_manager_setting_migrater_helper::EnsureFieldTrialSetup()) { - password_manager_setting_migrater_helper::InitializePreferencesMigration( - GetProfile(index)); - } - } - - void ExpectValueOnBothClientsForBothPreference(bool value) { - for (int i = 0; i < num_clients(); ++i) { - ExpectPrefValuesOnClient(i, value, value); - } - } - - Profile* profile() { return GetProfile(0); } - bool TestUsesSelfNotifications() override { return false; } - - private: - DISALLOW_COPY_AND_ASSIGN( - TwoClientsPasswordManagerSettingMigratorServiceSyncTest); -}; - -IN_PROC_BROWSER_TEST_F(TwoClientsPasswordManagerSettingMigratorServiceSyncTest, - E2E_ENABLED(ChangeLegacyPrefTestBothClientsWithMigration)) { - ASSERT_TRUE(SetupSync()); - EnsureMigrationStartsForClient(0); - EnsureMigrationStartsForClient(1); - TestPrefChangeOnClient(1, kPasswordManagerSavingEnabled); -} - -IN_PROC_BROWSER_TEST_F(TwoClientsPasswordManagerSettingMigratorServiceSyncTest, - E2E_ENABLED(ChangeNewPrefTestBothClientsWithMigration)) { - ASSERT_TRUE(SetupClients()); - EnsureMigrationStartsForClient(0); - EnsureMigrationStartsForClient(1); - ASSERT_TRUE(SetupSync()); - TestPrefChangeOnClient(1, kCredentialsEnableService); -} - -IN_PROC_BROWSER_TEST_F( - TwoClientsPasswordManagerSettingMigratorServiceSyncTest, - E2E_ENABLED(ChangeLegacyPrefOnMigratedClientOneClientsWithMigration)) { - ASSERT_TRUE(SetupSync()); - EnsureMigrationStartsForClient(0); - TestPrefChangeOnClient(0, kPasswordManagerSavingEnabled); - ExpectValueOnBothClientsForBothPreference(false); -} - -IN_PROC_BROWSER_TEST_F( - TwoClientsPasswordManagerSettingMigratorServiceSyncTest, - E2E_ENABLED(ChangeLegacyPrefOnNonMigratedClientOneClientsWithMigration)) { - ASSERT_TRUE(SetupSync()); - EnsureMigrationStartsForClient(0); - TestPrefChangeOnClient(1, kPasswordManagerSavingEnabled); - ExpectValueOnBothClientsForBothPreference(false); -} - -IN_PROC_BROWSER_TEST_F(TwoClientsPasswordManagerSettingMigratorServiceSyncTest, - E2E_ENABLED(ChangeNewPrefOnMigratedClientOneClientsWithMigration)) { - ASSERT_TRUE(SetupSync()); - EnsureMigrationStartsForClient(0); - TestPrefChangeOnClient(0, kCredentialsEnableService); - ExpectValueOnBothClientsForBothPreference(false); -} - -IN_PROC_BROWSER_TEST_F( - TwoClientsPasswordManagerSettingMigratorServiceSyncTest, - E2E_ENABLED(ChangeNewPrefOnNonMigratedClientOneClientsWithMigration)) { - ASSERT_TRUE(SetupSync()); - EnsureMigrationStartsForClient(0); - TestPrefChangeOnClient(1, kCredentialsEnableService); - ASSERT_TRUE(BooleanPrefMatchChecker(kPasswordManagerSavingEnabled).Wait()); - ExpectValueOnBothClientsForBothPreference(false); -}
diff --git a/chrome/browser/ui/passwords/manage_passwords_view_utils_desktop.cc b/chrome/browser/ui/passwords/manage_passwords_view_utils_desktop.cc index 2583b2c..ec95572 100644 --- a/chrome/browser/ui/passwords/manage_passwords_view_utils_desktop.cc +++ b/chrome/browser/ui/passwords/manage_passwords_view_utils_desktop.cc
@@ -6,15 +6,13 @@ #include "chrome/grit/generated_resources.h" #include "components/password_manager/core/browser/password_bubble_experiment.h" -#include "components/password_manager/core/browser/password_manager_settings_migration_experiment.h" int GetPasswordManagerSettingsStringId( const syncer::SyncService* sync_service) { int smart_lock_users_ids = IDS_OPTIONS_PASSWORD_MANAGER_SMART_LOCK_ENABLE; int non_smart_lock_users_ids = IDS_OPTIONS_PASSWORD_MANAGER_ENABLE; - if (password_bubble_experiment::IsSmartLockUser(sync_service) && - password_manager::IsSettingsMigrationActive()) { + if (password_bubble_experiment::IsSmartLockUser(sync_service)) { return smart_lock_users_ids; } return non_smart_lock_users_ids;
diff --git a/chrome/browser/ui/passwords/manage_passwords_view_utils_desktop_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_view_utils_desktop_unittest.cc index 0c50d673..7358872 100644 --- a/chrome/browser/ui/passwords/manage_passwords_view_utils_desktop_unittest.cc +++ b/chrome/browser/ui/passwords/manage_passwords_view_utils_desktop_unittest.cc
@@ -26,11 +26,6 @@ enum UserType { SMART_LOCK_USER, NON_SMART_LOCK_USER }; -const char kPasswordManagerSettingMigrationFieldTrialName[] = - "PasswordManagerSettingsMigration"; -const char kEnabledPasswordManagerSettingsMigrationGroupName[] = "Enable"; -const char kDisablePasswordManagerSettingsMigrationGroupName[] = "Disable"; - } // namespace class ManagePasswordsViewUtilDesktopTest : public testing::Test { @@ -45,11 +40,6 @@ Profile* profile() { return &profile_; } - void EnforcePasswordManagerSettingMigrationExperiment(const char* name) { - settings_migration_ = base::FieldTrialList::CreateFieldTrial( - kPasswordManagerSettingMigrationFieldTrialName, name); - } - browser_sync::ProfileSyncService* GetSyncServiceForSmartLockUser() { browser_sync::ProfileSyncServiceMock* sync_service = static_cast<browser_sync::ProfileSyncServiceMock*>( @@ -81,36 +71,19 @@ TEST_F(ManagePasswordsViewUtilDesktopTest, GetPasswordManagerSettingsStringId) { const struct { - const char* description; - const char* settings_migration_experiment_group; UserType user_type; int expected_setting_description_id; } kTestData[] = { - {"Smart Lock User, migration active", - kEnabledPasswordManagerSettingsMigrationGroupName, SMART_LOCK_USER, - IDS_OPTIONS_PASSWORD_MANAGER_SMART_LOCK_ENABLE}, - {"Smart Lock User, no migration", - kDisablePasswordManagerSettingsMigrationGroupName, SMART_LOCK_USER, - IDS_OPTIONS_PASSWORD_MANAGER_ENABLE}, - {"Non Smart Lock User, no migration", - kDisablePasswordManagerSettingsMigrationGroupName, NON_SMART_LOCK_USER, - IDS_OPTIONS_PASSWORD_MANAGER_ENABLE}, - {"Non Smart Lock User, migration", - kEnabledPasswordManagerSettingsMigrationGroupName, NON_SMART_LOCK_USER, - IDS_OPTIONS_PASSWORD_MANAGER_ENABLE}, + {SMART_LOCK_USER, IDS_OPTIONS_PASSWORD_MANAGER_SMART_LOCK_ENABLE}, + {NON_SMART_LOCK_USER, IDS_OPTIONS_PASSWORD_MANAGER_ENABLE}, }; for (const auto& test_case : kTestData) { - base::FieldTrialList field_trial_list( - base::MakeUnique<base::MockEntropyProvider>()); - SCOPED_TRACE(testing::Message(test_case.description)); browser_sync::ProfileSyncService* sync_service; if (test_case.user_type == SMART_LOCK_USER) sync_service = GetSyncServiceForSmartLockUser(); else sync_service = GetSyncServiceForNonSmartLockUser(); - EnforcePasswordManagerSettingMigrationExperiment( - test_case.settings_migration_experiment_group); EXPECT_EQ( l10n_util::GetStringUTF16(test_case.expected_setting_description_id), l10n_util::GetStringUTF16(
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc index acf909d..f7c2be4 100644 --- a/chrome/browser/ui/views/frame/browser_view.cc +++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -429,9 +429,6 @@ // OffTheRecordProfile's PrefService which gets deleted by ~Browser. RemoveAllChildViews(true); toolbar_ = nullptr; - - // Explicitly set browser_ to null. - browser_.reset(); } void BrowserView::Init(Browser* browser) {
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 41e6fa7..2b81188a 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -895,8 +895,6 @@ "../browser/sync/test/integration/p2p_invalidation_forwarder.h", "../browser/sync/test/integration/p2p_sync_refresher.cc", "../browser/sync/test/integration/p2p_sync_refresher.h", - "../browser/sync/test/integration/password_manager_setting_migrator_helper.cc", - "../browser/sync/test/integration/password_manager_setting_migrator_helper.h", "../browser/sync/test/integration/passwords_helper.cc", "../browser/sync/test/integration/passwords_helper.h", "../browser/sync/test/integration/preferences_helper.cc", @@ -2647,7 +2645,6 @@ "../browser/sync/test/integration/single_client_dictionary_sync_test.cc", "../browser/sync/test/integration/single_client_directory_sync_test.cc", "../browser/sync/test/integration/single_client_extensions_sync_test.cc", - "../browser/sync/test/integration/single_client_password_manager_setting_migrator_service_sync_test.cc", "../browser/sync/test/integration/single_client_passwords_sync_test.cc", "../browser/sync/test/integration/single_client_preferences_sync_test.cc", "../browser/sync/test/integration/single_client_printers_sync_test.cc", @@ -2669,7 +2666,6 @@ "../browser/sync/test/integration/two_client_dictionary_sync_test.cc", "../browser/sync/test/integration/two_client_extension_settings_and_app_settings_sync_test.cc", "../browser/sync/test/integration/two_client_extensions_sync_test.cc", - "../browser/sync/test/integration/two_client_password_manager_setting_migrator_service_sync_test.cc", "../browser/sync/test/integration/two_client_passwords_sync_test.cc", "../browser/sync/test/integration/two_client_preferences_sync_test.cc", "../browser/sync/test/integration/two_client_printers_sync_test.cc",
diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc index f67cb424..1faa6b0e 100644 --- a/chrome/test/base/testing_profile.cc +++ b/chrome/test/base/testing_profile.cc
@@ -828,7 +828,8 @@ // Simplified version of ProfileImpl::GetOffTheRecordPrefs(). Note this // leaves testing_prefs_ unset. prefs_.reset(CreateIncognitoPrefServiceSyncable( - original_profile_->prefs_.get(), NULL)); + original_profile_->prefs_.get(), nullptr, + std::set<PrefValueStore::PrefStoreType>(), nullptr, nullptr)); user_prefs::UserPrefs::Set(this, prefs_.get()); }
diff --git a/components/bookmarks/browser/bookmark_storage.cc b/components/bookmarks/browser/bookmark_storage.cc index d32a90c..ed52b77 100644 --- a/components/bookmarks/browser/bookmark_storage.cc +++ b/components/bookmarks/browser/bookmark_storage.cc
@@ -153,10 +153,10 @@ : model_(model), writer_(profile_path.Append(kBookmarksFileName), sequenced_task_runner, - base::TimeDelta::FromMilliseconds(kSaveDelayMS)), + base::TimeDelta::FromMilliseconds(kSaveDelayMS), + "BookmarkStorage"), sequenced_task_runner_(sequenced_task_runner), - weak_factory_(this) { -} + weak_factory_(this) {} BookmarkStorage::~BookmarkStorage() { if (writer_.HasPendingWrite())
diff --git a/components/feedback/feedback_report.cc b/components/feedback/feedback_report.cc index fa0e4631..bdbd469 100644 --- a/components/feedback/feedback_report.cc +++ b/components/feedback/feedback_report.cc
@@ -29,7 +29,7 @@ if (!base::CreateDirectoryAndGetError(reports_path, &error)) return; } - base::ImportantFileWriter::WriteFileAtomically(file, data); + base::ImportantFileWriter::WriteFileAtomically(file, data, "FeedbackReport"); } } // namespace
diff --git a/components/password_manager/core/browser/BUILD.gn b/components/password_manager/core/browser/BUILD.gn index e635a07..ac47b07b 100644 --- a/components/password_manager/core/browser/BUILD.gn +++ b/components/password_manager/core/browser/BUILD.gn
@@ -99,8 +99,6 @@ "password_manager_internals_service.h", "password_manager_metrics_util.cc", "password_manager_metrics_util.h", - "password_manager_settings_migration_experiment.cc", - "password_manager_settings_migration_experiment.h", "password_manager_util.cc", "password_manager_util.h", "password_store.cc", @@ -315,7 +313,6 @@ "password_form_manager_unittest.cc", "password_form_metrics_recorder_unittest.cc", "password_generation_manager_unittest.cc", - "password_manager_settings_migration_experiment_unittest.cc", "password_manager_unittest.cc", "password_manager_util_unittest.cc", "password_store_default_unittest.cc",
diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc index d38e2279..78ff536 100644 --- a/components/password_manager/core/browser/password_form_manager.cc +++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -252,7 +252,9 @@ PasswordFormManager::~PasswordFormManager() { form_fetcher_->RemoveConsumer(this); - RecordHistogramsOnSuppressedAccounts(); + metrics_recorder_.RecordHistogramsOnSuppressedAccounts( + observed_form_.origin.SchemeIsCryptographic(), *form_fetcher_, + pending_credentials_); if (form_type_ != kFormTypeUnspecified) { UMA_HISTOGRAM_ENUMERATION("PasswordManager.SubmittedFormType", form_type_, @@ -264,87 +266,6 @@ } } -int PasswordFormManager::GetHistogramSampleForSuppressedAccounts( - const std::vector<const autofill::PasswordForm*> suppressed_forms, - PasswordForm::Type manual_or_generated) const { - DCHECK(form_fetcher_->DidCompleteQueryingSuppressedForms()); - - SuppressedAccountExistence best_matching_account = kSuppressedAccountNone; - for (const autofill::PasswordForm* form : suppressed_forms) { - if (form->type != manual_or_generated) - continue; - - SuppressedAccountExistence current_account; - if (pending_credentials_.password_value.empty()) - current_account = kSuppressedAccountExists; - else if (form->username_value != pending_credentials_.username_value) - current_account = kSuppressedAccountExistsDifferentUsername; - else if (form->password_value != pending_credentials_.password_value) - current_account = kSuppressedAccountExistsSameUsername; - else - current_account = kSuppressedAccountExistsSameUsernameAndPassword; - - best_matching_account = std::max(best_matching_account, current_account); - } - - // Encoding: most significant digit is the |best_matching_account|. - int mixed_base_encoding = 0; - mixed_base_encoding += best_matching_account; - mixed_base_encoding *= PasswordFormMetricsRecorder::kMaxNumActionsTakenNew; - mixed_base_encoding += metrics_recorder_.GetActionsTakenNew(); - DCHECK_LT(mixed_base_encoding, kMaxSuppressedAccountStats); - return mixed_base_encoding; -} - -void PasswordFormManager::RecordHistogramsOnSuppressedAccounts() const { - UMA_HISTOGRAM_BOOLEAN("PasswordManager.QueryingSuppressedAccountsFinished", - form_fetcher_->DidCompleteQueryingSuppressedForms()); - - if (!form_fetcher_->DidCompleteQueryingSuppressedForms()) - return; - - if (!observed_form_.origin.SchemeIsCryptographic()) { - UMA_HISTOGRAM_ENUMERATION( - "PasswordManager.SuppressedAccount.Generated.HTTPSNotHTTP", - GetHistogramSampleForSuppressedAccounts( - form_fetcher_->GetSuppressedHTTPSForms(), - PasswordForm::TYPE_GENERATED), - kMaxSuppressedAccountStats); - UMA_HISTOGRAM_ENUMERATION( - "PasswordManager.SuppressedAccount.Manual.HTTPSNotHTTP", - GetHistogramSampleForSuppressedAccounts( - form_fetcher_->GetSuppressedHTTPSForms(), - PasswordForm::TYPE_MANUAL), - kMaxSuppressedAccountStats); - } - - UMA_HISTOGRAM_ENUMERATION( - "PasswordManager.SuppressedAccount.Generated.PSLMatching", - GetHistogramSampleForSuppressedAccounts( - form_fetcher_->GetSuppressedPSLMatchingForms(), - PasswordForm::TYPE_GENERATED), - kMaxSuppressedAccountStats); - UMA_HISTOGRAM_ENUMERATION( - "PasswordManager.SuppressedAccount.Manual.PSLMatching", - GetHistogramSampleForSuppressedAccounts( - form_fetcher_->GetSuppressedPSLMatchingForms(), - PasswordForm::TYPE_MANUAL), - kMaxSuppressedAccountStats); - - UMA_HISTOGRAM_ENUMERATION( - "PasswordManager.SuppressedAccount.Generated.SameOrganizationName", - GetHistogramSampleForSuppressedAccounts( - form_fetcher_->GetSuppressedSameOrganizationNameForms(), - PasswordForm::TYPE_GENERATED), - kMaxSuppressedAccountStats); - UMA_HISTOGRAM_ENUMERATION( - "PasswordManager.SuppressedAccount.Manual.SameOrganizationName", - GetHistogramSampleForSuppressedAccounts( - form_fetcher_->GetSuppressedSameOrganizationNameForms(), - PasswordForm::TYPE_MANUAL), - kMaxSuppressedAccountStats); -} - // static base::string16 PasswordFormManager::PasswordToSave(const PasswordForm& form) { if (form.new_password_element.empty() || form.new_password_value.empty())
diff --git a/components/password_manager/core/browser/password_form_manager.h b/components/password_manager/core/browser/password_form_manager.h index 0eac86b3..e1e00b3 100644 --- a/components/password_manager/core/browser/password_form_manager.h +++ b/components/password_manager/core/browser/password_form_manager.h
@@ -283,32 +283,6 @@ kFoundGenerationElement }; - // Enumerates whether there were `suppressed` credentials. These are stored - // credentials that were not filled, even though they might be related to the - // |observed_form_|. See FormFetcher::GetSuppressed* for details. - // - // If suppressed credentials exist, it is also recorded whether their username - // and/or password matched those submitted. - enum SuppressedAccountExistence { - kSuppressedAccountNone, - // Recorded when there exists a suppressed account, but there was no - // submitted form to compare its username and password to. - kSuppressedAccountExists, - // Recorded when there was a submitted form. - kSuppressedAccountExistsDifferentUsername, - kSuppressedAccountExistsSameUsername, - kSuppressedAccountExistsSameUsernameAndPassword, - kSuppressedAccountExistenceMax, - }; - - // The maximum number of combinations recorded into histograms in the - // PasswordManager.SuppressedAccount.* family. - static constexpr int kMaxSuppressedAccountStats = - kSuppressedAccountExistenceMax * - PasswordFormMetricsRecorder::kManagerActionNewMax * - static_cast<int>(UserAction::kMax) * - PasswordFormMetricsRecorder::kSubmitResultMax; - // Through |driver|, supply the associated frame with appropriate information // (fill data, whether to allow password generation, etc.). void ProcessFrameInternal(const base::WeakPtr<PasswordManagerDriver>& driver); @@ -379,23 +353,6 @@ bool UpdatePendingCredentialsIfUsernameChanged( const autofill::PasswordForm& form); - // When supplied with the list of all |suppressed_forms| that belong to - // certain suppressed credential type (see FormFetcher::GetSuppressed*), - // filters that list down to forms that are either |manual_or_generated|, and - // based on that, computes the histogram sample that is a mixed-based - // representation of a combination of four attributes: - // -- whether there were suppressed credentials (and if so, their relation to - // the submitted username/password). - // -- whether the |observed_form_| got ultimately submitted - // -- what action the password manager performed (|manager_action_|), - // -- and what action the user performed (|user_action_|_). - int GetHistogramSampleForSuppressedAccounts( - const std::vector<const autofill::PasswordForm*> suppressed_forms, - autofill::PasswordForm::Type manual_or_generated) const; - - // Records all histograms in the PasswordManager.SuppressedAccount.* family. - void RecordHistogramsOnSuppressedAccounts() const; - // Tries to set all votes (e.g. autofill field types, generation vote) to // a |FormStructure| and upload it to the server. Returns true on success. bool UploadPasswordVote(const autofill::PasswordForm& form_to_upload,
diff --git a/components/password_manager/core/browser/password_form_metrics_recorder.cc b/components/password_manager/core/browser/password_form_metrics_recorder.cc index 0ee2b61..8097db0 100644 --- a/components/password_manager/core/browser/password_form_metrics_recorder.cc +++ b/components/password_manager/core/browser/password_form_metrics_recorder.cc
@@ -4,11 +4,16 @@ #include "components/password_manager/core/browser/password_form_metrics_recorder.h" +#include <algorithm> + #include "base/logging.h" #include "base/metrics/histogram_macros.h" #include "base/metrics/user_metrics.h" +#include "components/password_manager/core/browser/form_fetcher.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h" +using autofill::PasswordForm; + namespace password_manager { PasswordFormMetricsRecorder::PasswordFormMetricsRecorder( @@ -116,4 +121,87 @@ (manager_action_ + kManagerActionMax * submit_result_); } +void PasswordFormMetricsRecorder::RecordHistogramsOnSuppressedAccounts( + bool observed_form_origin_has_cryptographic_scheme, + const FormFetcher& form_fetcher, + const PasswordForm& pending_credentials) const { + UMA_HISTOGRAM_BOOLEAN("PasswordManager.QueryingSuppressedAccountsFinished", + form_fetcher.DidCompleteQueryingSuppressedForms()); + + if (!form_fetcher.DidCompleteQueryingSuppressedForms()) + return; + + if (!observed_form_origin_has_cryptographic_scheme) { + UMA_HISTOGRAM_ENUMERATION( + "PasswordManager.SuppressedAccount.Generated.HTTPSNotHTTP", + GetHistogramSampleForSuppressedAccounts( + form_fetcher.GetSuppressedHTTPSForms(), + PasswordForm::TYPE_GENERATED, pending_credentials), + kMaxSuppressedAccountStats); + UMA_HISTOGRAM_ENUMERATION( + "PasswordManager.SuppressedAccount.Manual.HTTPSNotHTTP", + GetHistogramSampleForSuppressedAccounts( + form_fetcher.GetSuppressedHTTPSForms(), PasswordForm::TYPE_MANUAL, + pending_credentials), + kMaxSuppressedAccountStats); + } + + UMA_HISTOGRAM_ENUMERATION( + "PasswordManager.SuppressedAccount.Generated.PSLMatching", + GetHistogramSampleForSuppressedAccounts( + form_fetcher.GetSuppressedPSLMatchingForms(), + PasswordForm::TYPE_GENERATED, pending_credentials), + kMaxSuppressedAccountStats); + UMA_HISTOGRAM_ENUMERATION( + "PasswordManager.SuppressedAccount.Manual.PSLMatching", + GetHistogramSampleForSuppressedAccounts( + form_fetcher.GetSuppressedPSLMatchingForms(), + PasswordForm::TYPE_MANUAL, pending_credentials), + kMaxSuppressedAccountStats); + + UMA_HISTOGRAM_ENUMERATION( + "PasswordManager.SuppressedAccount.Generated.SameOrganizationName", + GetHistogramSampleForSuppressedAccounts( + form_fetcher.GetSuppressedSameOrganizationNameForms(), + PasswordForm::TYPE_GENERATED, pending_credentials), + kMaxSuppressedAccountStats); + UMA_HISTOGRAM_ENUMERATION( + "PasswordManager.SuppressedAccount.Manual.SameOrganizationName", + GetHistogramSampleForSuppressedAccounts( + form_fetcher.GetSuppressedSameOrganizationNameForms(), + PasswordForm::TYPE_MANUAL, pending_credentials), + kMaxSuppressedAccountStats); +} + +int PasswordFormMetricsRecorder::GetHistogramSampleForSuppressedAccounts( + const std::vector<const PasswordForm*>& suppressed_forms, + PasswordForm::Type manual_or_generated, + const PasswordForm& pending_credentials) const { + SuppressedAccountExistence best_matching_account = kSuppressedAccountNone; + for (const PasswordForm* form : suppressed_forms) { + if (form->type != manual_or_generated) + continue; + + SuppressedAccountExistence current_account; + if (pending_credentials.password_value.empty()) + current_account = kSuppressedAccountExists; + else if (form->username_value != pending_credentials.username_value) + current_account = kSuppressedAccountExistsDifferentUsername; + else if (form->password_value != pending_credentials.password_value) + current_account = kSuppressedAccountExistsSameUsername; + else + current_account = kSuppressedAccountExistsSameUsernameAndPassword; + + best_matching_account = std::max(best_matching_account, current_account); + } + + // Encoding: most significant digit is the |best_matching_account|. + int mixed_base_encoding = 0; + mixed_base_encoding += best_matching_account; + mixed_base_encoding *= PasswordFormMetricsRecorder::kMaxNumActionsTakenNew; + mixed_base_encoding += GetActionsTakenNew(); + DCHECK_LT(mixed_base_encoding, kMaxSuppressedAccountStats); + return mixed_base_encoding; +} + } // namespace password_manager
diff --git a/components/password_manager/core/browser/password_form_metrics_recorder.h b/components/password_manager/core/browser/password_form_metrics_recorder.h index cf15972..e17f679f 100644 --- a/components/password_manager/core/browser/password_form_metrics_recorder.h +++ b/components/password_manager/core/browser/password_form_metrics_recorder.h
@@ -5,12 +5,16 @@ #ifndef COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_FORM_METRICS_RECORDER_H_ #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_FORM_METRICS_RECORDER_H_ -#include "base/macros.h" +#include <vector> +#include "base/macros.h" +#include "components/autofill/core/common/password_form.h" #include "components/password_manager/core/browser/password_form_user_action.h" namespace password_manager { +class FormFetcher; + // The pupose of this class is to record various types of metrics about the // behavior of the PasswordFormManager and its interaction with the user and the // page. @@ -46,6 +50,24 @@ kSubmitResultMax }; + // Enumerates whether there were `suppressed` credentials. These are stored + // credentials that were not filled, even though they might be related to the + // observed form. See FormFetcher::GetSuppressed* for details. + // + // If suppressed credentials exist, it is also recorded whether their username + // and/or password matched those submitted. + enum SuppressedAccountExistence { + kSuppressedAccountNone, + // Recorded when there exists a suppressed account, but there was no + // submitted form to compare its username and password to. + kSuppressedAccountExists, + // Recorded when there was a submitted form. + kSuppressedAccountExistsDifferentUsername, + kSuppressedAccountExistsSameUsername, + kSuppressedAccountExistsSameUsernameAndPassword, + kSuppressedAccountExistenceMax, + }; + // The maximum number of combinations of the ManagerAction, UserAction and // SubmitResult enums. // This is used when recording the actions taken by the form in UMA. @@ -57,6 +79,14 @@ kManagerActionNewMax * static_cast<int>(UserAction::kMax) * kSubmitResultMax; + // The maximum number of combinations recorded into histograms in the + // PasswordManager.SuppressedAccount.* family. + static constexpr int kMaxSuppressedAccountStats = + kSuppressedAccountExistenceMax * + PasswordFormMetricsRecorder::kManagerActionNewMax * + static_cast<int>(UserAction::kMax) * + PasswordFormMetricsRecorder::kSubmitResultMax; + // Called if the user could generate a password for this form. void MarkGenerationAvailable(); @@ -72,9 +102,19 @@ void LogSubmitPassed(); void LogSubmitFailed(); + // Records all histograms in the PasswordManager.SuppressedAccount.* family. + // Takes the FormFetcher intance which owns the login data from PasswordStore. + // |pending_credentials| stores credentials when the form was submitted but + // success was still unknown. It contains credentials that are ready to be + // written (saved or updated) to a password store. + void RecordHistogramsOnSuppressedAccounts( + bool observed_form_origin_has_cryptographic_scheme, + const FormFetcher& form_fetcher, + const autofill::PasswordForm& pending_credentials) const; + // Converts the "ActionsTaken" fields (using ManagerActionNew) into an int so // they can be logged to UMA. - // TODO(battre): This should not be public. + // Public for testing. int GetActionsTakenNew() const; private: @@ -82,6 +122,24 @@ // UMA. int GetActionsTaken() const; + // When supplied with the list of all |suppressed_forms| that belong to + // certain suppressed credential type (see FormFetcher::GetSuppressed*), + // filters that list down to forms that are either |manual_or_generated|, and + // based on that, computes the histogram sample that is a mixed-based + // representation of a combination of four attributes: + // -- whether there were suppressed credentials (and if so, their relation to + // the submitted username/password). + // -- whether the |observed_form_| got ultimately submitted + // -- what action the password manager performed (|manager_action_|), + // -- and what action the user performed (|user_action_|_). + // |pending_credentials| stores credentials when the form was submitted but + // success was still unknown. It contains credentials that are ready to be + // written (saved or updated) to a password store. + int GetHistogramSampleForSuppressedAccounts( + const std::vector<const autofill::PasswordForm*>& suppressed_forms, + autofill::PasswordForm::Type manual_or_generated, + const autofill::PasswordForm& pending_credentials) const; + // True if the main frame's visible URL, at the time this PasswordFormManager // was created, is secure. const bool is_main_frame_secure_;
diff --git a/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc b/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc index 30da6b4..d2724ca 100644 --- a/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc +++ b/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc
@@ -122,7 +122,7 @@ // Test the recording of metrics around manager_action, user_action, and // submit_result. TEST(PasswordFormMetricsRecorder, Actions) { - static constexpr const struct { + static constexpr struct { // Stimuli: bool is_main_frame_secure; PasswordFormMetricsRecorder::ManagerAction manager_action;
diff --git a/components/password_manager/core/browser/password_manager_settings_migration_experiment.cc b/components/password_manager/core/browser/password_manager_settings_migration_experiment.cc deleted file mode 100644 index 28041664..0000000 --- a/components/password_manager/core/browser/password_manager_settings_migration_experiment.cc +++ /dev/null
@@ -1,20 +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. - -#include "components/password_manager/core/browser/password_manager_settings_migration_experiment.h" - -#include "base/metrics/field_trial.h" -#include "base/strings/string_util.h" - -namespace password_manager { - -bool IsSettingsMigrationActive() { - const char kFieldTrialName[] = "PasswordManagerSettingsMigration"; - const char kEnabledGroupNamePrefix[] = "Enable"; - return base::StartsWith(base::FieldTrialList::FindFullName(kFieldTrialName), - kEnabledGroupNamePrefix, - base::CompareCase::INSENSITIVE_ASCII); -} - -} // namespace password_manager
diff --git a/components/password_manager/core/browser/password_manager_settings_migration_experiment.h b/components/password_manager/core/browser/password_manager_settings_migration_experiment.h deleted file mode 100644 index 0dcbd1cb..0000000 --- a/components/password_manager/core/browser/password_manager_settings_migration_experiment.h +++ /dev/null
@@ -1,15 +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 COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_SETTINGS_MIGRATOR_EXPERIMENT_H_ -#define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_SETTINGS_MIGRATOR_EXPERIMENT_H_ - -namespace password_manager { - -// Returns true if settings migration should be active for the user. -bool IsSettingsMigrationActive(); - -} // namespace password_manager - -#endif // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_SETTINGS_MIGRATOR_EXPERIMENT_H_
diff --git a/components/password_manager/core/browser/password_manager_settings_migration_experiment_unittest.cc b/components/password_manager/core/browser/password_manager_settings_migration_experiment_unittest.cc deleted file mode 100644 index 81d8d42..0000000 --- a/components/password_manager/core/browser/password_manager_settings_migration_experiment_unittest.cc +++ /dev/null
@@ -1,51 +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. - -#include "components/password_manager/core/browser/password_manager_settings_migration_experiment.h" - -#include "base/macros.h" -#include "base/metrics/field_trial.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace { - -const char kPasswordManagerSettingMigrationFieldTrialName[] = - "PasswordManagerSettingsMigration"; -const char kEnabledPasswordManagerSettingsMigrationGroupName[] = "Enable"; -const char kDisablePasswordManagerSettingsMigrationGroupName[] = "Disable"; - -} // namespace - -namespace password_manager { - -class PasswordManagerSettingsMigrationExperimentTest : public testing::Test { - public: - PasswordManagerSettingsMigrationExperimentTest() - : field_trial_list_(nullptr) {} - - void EnforcePasswordManagerSettingMigrationExperimentGroup(const char* name) { - ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial( - kPasswordManagerSettingMigrationFieldTrialName, name)); - } - - protected: - base::FieldTrialList field_trial_list_; - - DISALLOW_COPY_AND_ASSIGN(PasswordManagerSettingsMigrationExperimentTest); -}; - -TEST_F(PasswordManagerSettingsMigrationExperimentTest, IsSettingsMigrationOn) { - EnforcePasswordManagerSettingMigrationExperimentGroup( - kEnabledPasswordManagerSettingsMigrationGroupName); - EXPECT_TRUE(IsSettingsMigrationActive()); -} - -TEST_F(PasswordManagerSettingsMigrationExperimentTest, IsSettingsMigrationOff) { - EnforcePasswordManagerSettingMigrationExperimentGroup( - kDisablePasswordManagerSettingsMigrationGroupName); - EXPECT_FALSE(IsSettingsMigrationActive()); -} - -} // namespace password_manager
diff --git a/components/password_manager/sync/browser/BUILD.gn b/components/password_manager/sync/browser/BUILD.gn index 88b02904..77c38bc 100644 --- a/components/password_manager/sync/browser/BUILD.gn +++ b/components/password_manager/sync/browser/BUILD.gn
@@ -6,8 +6,6 @@ sources = [ "password_data_type_controller.cc", "password_data_type_controller.h", - "password_manager_setting_migrator_service.cc", - "password_manager_setting_migrator_service.h", "password_model_worker.cc", "password_model_worker.h", "password_sync_util.cc", @@ -35,7 +33,6 @@ source_set("unit_tests") { testonly = true sources = [ - "password_manager_setting_migrator_service_unittest.cc", "password_sync_util_unittest.cc", "sync_credentials_filter_unittest.cc", "sync_username_test_base.cc",
diff --git a/components/password_manager/sync/browser/password_manager_setting_migrator_service.cc b/components/password_manager/sync/browser/password_manager_setting_migrator_service.cc deleted file mode 100644 index 28147ea..0000000 --- a/components/password_manager/sync/browser/password_manager_setting_migrator_service.cc +++ /dev/null
@@ -1,238 +0,0 @@ -// Copyright (c) 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. - -#include "components/password_manager/sync/browser/password_manager_setting_migrator_service.h" - -#include "base/bind.h" -#include "base/bind_helpers.h" -#include "base/metrics/histogram_macros.h" -#include "build/build_config.h" -#include "components/password_manager/core/browser/password_manager_settings_migration_experiment.h" -#include "components/password_manager/core/common/password_manager_pref_names.h" -#include "components/sync/driver/sync_service.h" -#include "components/sync_preferences/pref_service_syncable.h" - -namespace { - -bool GetBooleanUserOrDefaultPrefValue(PrefService* prefs, - const std::string& name) { - bool result = false; - const base::Value* value = prefs->GetUserPrefValue(name); - if (!value) - value = prefs->GetDefaultPrefValue(name); - value->GetAsBoolean(&result); - return result; -} - -void ChangeOnePrefBecauseAnotherPrefHasChanged( - PrefService* prefs, - const std::string& other_pref_name, - const std::string& changed_pref_name) { - bool changed_pref = - GetBooleanUserOrDefaultPrefValue(prefs, changed_pref_name); - bool other_pref = GetBooleanUserOrDefaultPrefValue(prefs, other_pref_name); - if (changed_pref != other_pref) - prefs->SetBoolean(other_pref_name, changed_pref); -} - -// Change value of both kPasswordManagerSavingEnabled and -// kCredentialsEnableService to the |new_value|. -void UpdatePreferencesValues(PrefService* prefs, bool new_value) { - prefs->SetBoolean(password_manager::prefs::kPasswordManagerSavingEnabled, - new_value); - prefs->SetBoolean(password_manager::prefs::kCredentialsEnableService, - new_value); -} - -void SaveCurrentPrefState(PrefService* prefs, - bool* new_pref_value, - bool* legacy_pref_value) { - *new_pref_value = GetBooleanUserOrDefaultPrefValue( - prefs, password_manager::prefs::kCredentialsEnableService); - *legacy_pref_value = GetBooleanUserOrDefaultPrefValue( - prefs, password_manager::prefs::kPasswordManagerSavingEnabled); -} - -void TrackInitialAndFinalValues(PrefService* prefs, - bool initial_new_pref_value, - bool initial_legacy_pref_value) { - bool final_new_pref_value = GetBooleanUserOrDefaultPrefValue( - prefs, password_manager::prefs::kCredentialsEnableService); - bool final_legacy_pref_value = GetBooleanUserOrDefaultPrefValue( - prefs, password_manager::prefs::kPasswordManagerSavingEnabled); - const int kMaxInitValue = 0x10; - int value_to_log = 0; - const int kInitialNewValueMask = 0x8; - const int kInitialLegacyValueMask = 0x4; - const int kFinalNewValueMask = 0x2; - const int kFinalLegacyValueMask = 0x1; - if (initial_new_pref_value) - value_to_log |= kInitialNewValueMask; - if (initial_legacy_pref_value) - value_to_log |= kInitialLegacyValueMask; - if (final_new_pref_value) - value_to_log |= kFinalNewValueMask; - if (final_legacy_pref_value) - value_to_log |= kFinalLegacyValueMask; - UMA_HISTOGRAM_ENUMERATION( - "PasswordManager.SettingsReconciliation.InitialAndFinalValues", - value_to_log, kMaxInitValue); -} - -} // namespace - -namespace password_manager { - -bool PasswordManagerSettingMigratorService::force_disabled_for_testing_ = false; - -PasswordManagerSettingMigratorService::PasswordManagerSettingMigratorService( - sync_preferences::PrefServiceSyncable* prefs) - : prefs_(prefs), sync_service_(nullptr) { - SaveCurrentPrefState(prefs_, &initial_new_pref_value_, - &initial_legacy_pref_value_); -} - -PasswordManagerSettingMigratorService:: - ~PasswordManagerSettingMigratorService() {} - -void PasswordManagerSettingMigratorService::InitializeMigration( - syncer::SyncService* sync_service) { - if (force_disabled_for_testing_) - return; - SaveCurrentPrefState(prefs_, &initial_new_pref_value_, - &initial_legacy_pref_value_); - const int kMaxInitialValues = 4; - UMA_HISTOGRAM_ENUMERATION( - "PasswordManager.SettingsReconciliation.InitialValues", - (static_cast<int>(initial_new_pref_value_) << 1 | - static_cast<int>(initial_legacy_pref_value_)), - kMaxInitialValues); - if (!password_manager::IsSettingsMigrationActive()) { - return; - } - sync_service_ = sync_service; - if (!sync_service_ || !CanSyncStart()) { - MigrateOffState(prefs_); - TrackInitialAndFinalValues(prefs_, initial_new_pref_value_, - initial_legacy_pref_value_); - } - InitObservers(); -} - -void PasswordManagerSettingMigratorService::InitObservers() { - pref_change_registrar_.Init(prefs_); - pref_change_registrar_.Add( - password_manager::prefs::kCredentialsEnableService, - base::Bind(&PasswordManagerSettingMigratorService:: - OnCredentialsEnableServicePrefChanged, - base::Unretained(this))); - pref_change_registrar_.Add( - password_manager::prefs::kPasswordManagerSavingEnabled, - base::Bind(&PasswordManagerSettingMigratorService:: - OnPasswordManagerSavingEnabledPrefChanged, - base::Unretained(this))); - // This causes OnIsSyncingChanged to be called when the value of - // PrefService::IsSyncing() changes. - prefs_->AddObserver(this); -} - -bool PasswordManagerSettingMigratorService::CanSyncStart() { - return sync_service_ && sync_service_->CanSyncStart() && - syncer::UserSelectableTypes().Has(syncer::PREFERENCES); -} - -void PasswordManagerSettingMigratorService::Shutdown() { - prefs_->RemoveObserver(this); -} - -void PasswordManagerSettingMigratorService:: - OnCredentialsEnableServicePrefChanged( - const std::string& changed_pref_name) { - sync_data_.push_back(GetBooleanUserOrDefaultPrefValue( - prefs_, password_manager::prefs::kCredentialsEnableService)); - ChangeOnePrefBecauseAnotherPrefHasChanged( - prefs_, password_manager::prefs::kPasswordManagerSavingEnabled, - password_manager::prefs::kCredentialsEnableService); -} - -void PasswordManagerSettingMigratorService:: - OnPasswordManagerSavingEnabledPrefChanged( - const std::string& changed_pref_name) { - sync_data_.push_back(GetBooleanUserOrDefaultPrefValue( - prefs_, password_manager::prefs::kPasswordManagerSavingEnabled)); - ChangeOnePrefBecauseAnotherPrefHasChanged( - prefs_, password_manager::prefs::kCredentialsEnableService, - password_manager::prefs::kPasswordManagerSavingEnabled); -} - -void PasswordManagerSettingMigratorService::OnIsSyncingChanged() { - if (WasModelAssociationStepPerformed()) { - // Initial sync has finished. - MigrateAfterModelAssociation(prefs_); - } - - if (prefs_->IsSyncing() == prefs_->IsPrioritySyncing()) { - // Sync is not in model association step. - SaveCurrentPrefState(prefs_, &initial_new_pref_value_, - &initial_legacy_pref_value_); - sync_data_.clear(); - } -} - -bool PasswordManagerSettingMigratorService::WasModelAssociationStepPerformed() { -#if defined(OS_ANDROID) || defined(OS_IOS) - return prefs_->IsPrioritySyncing(); -#else - return prefs_->IsSyncing() && prefs_->IsPrioritySyncing(); -#endif -} - -void PasswordManagerSettingMigratorService::MigrateOffState( - PrefService* prefs) { - bool new_pref_value = GetBooleanUserOrDefaultPrefValue( - prefs_, password_manager::prefs::kCredentialsEnableService); - bool legacy_pref_value = GetBooleanUserOrDefaultPrefValue( - prefs_, password_manager::prefs::kPasswordManagerSavingEnabled); - UpdatePreferencesValues(prefs, new_pref_value && legacy_pref_value); -} - -void PasswordManagerSettingMigratorService::MigrateAfterModelAssociation( - PrefService* prefs) { - if (sync_data_.empty()) { - MigrateOffState(prefs); - } else if (sync_data_.size() == 1) { -#if defined(OS_ANDROID) || defined(OS_IOS) - if (initial_new_pref_value_ != initial_legacy_pref_value_) { - // Treat special case for mobile clients where only priority pref - // arrives on the client. - if (!initial_new_pref_value_ && initial_legacy_pref_value_) - UpdatePreferencesValues(prefs, true); - else - UpdatePreferencesValues(prefs, false); - } else { - UpdatePreferencesValues(prefs, sync_data_[0]); - } -#else - // Only one value came from sync. This value should be assigned to both - // preferences. - UpdatePreferencesValues(prefs, sync_data_[0]); -#endif - } else { - bool sync_new_pref_value = sync_data_[0]; - bool sync_legacy_pref_value = sync_data_.back(); - if (sync_legacy_pref_value && sync_new_pref_value) { - UpdatePreferencesValues(prefs, true); - } else if (!sync_legacy_pref_value && !sync_new_pref_value) { - UpdatePreferencesValues(prefs, false); - } else if (!initial_legacy_pref_value_ && !initial_new_pref_value_) { - UpdatePreferencesValues(prefs, true); - } else { - UpdatePreferencesValues(prefs, false); - } - } - TrackInitialAndFinalValues(prefs, initial_new_pref_value_, - initial_legacy_pref_value_); -} - -} // namespace password_manager
diff --git a/components/password_manager/sync/browser/password_manager_setting_migrator_service.h b/components/password_manager/sync/browser/password_manager_setting_migrator_service.h deleted file mode 100644 index 797d11c..0000000 --- a/components/password_manager/sync/browser/password_manager_setting_migrator_service.h +++ /dev/null
@@ -1,165 +0,0 @@ -// Copyright (c) 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 COMPONENTS_PASSWORD_MANAGER_SYNC_BROWSER_PASSWORD_MANAGER_SETTING_MIGRATOR_SERVICE_H_ -#define COMPONENTS_PASSWORD_MANAGER_SYNC_BROWSER_PASSWORD_MANAGER_SETTING_MIGRATOR_SERVICE_H_ - -#include <string> -#include <vector> - -#include "base/macros.h" -#include "base/memory/singleton.h" -#include "components/keyed_service/core/keyed_service.h" -#include "components/prefs/pref_change_registrar.h" -#include "components/sync_preferences/pref_service_syncable_observer.h" - -namespace syncer { -class SyncService; -} - -namespace sync_preferences { -class PrefServiceSyncable; -} - -namespace password_manager { - -// Service that is responsible for reconciling the legacy "Offer to save your -// web passwords" setting (henceforth denoted 'L', for legacy) with the new -// "Enable Smart Lock for Passwords" setting (henceforth denoted 'N', for new). -// -// It works as follows. -// -// Users who are not syncing (this is checked on start-up of the service -// calling to SyncService:CanSyncStart()) are migrated on startup of the -// service. These users can be in following states: -// 1. N = 0, L = 0 -// 2. N = 1, L = 1 -// 3. N = 1, L = 0 -// 4. N = 0, L = 1 -// -// Nothing should be done in case 1 and 2 since settings are already in sync; -// in case 3,4 we change value of N to 0. -// -// For users who are syncing we save the values for the new and legacy -// preference on service startup (|inital_values_|) and the values that come -// from sync during models association step (|sync_data_|). Propagating change -// of one preference to another preference without having this special treatment -// will not always work. Here is an example which illustrates possible corner -// case: the client executes the migration code for the first time, legacy -// preference has a value 1, new preference was registered for the -// first time and has default value which is also 1, the sync data snapshot -// which user has on a client contains new preference equal to 0 and old -// preference equal to 1, if we blindly propagate these values we first get -// both preferences equal to 0 after priority pref model was associated and -// then both preferences equal to 1 after preferences model was associated. -// But the correct final value in this case is 0. -// -// At the end of model association step we derive the new values for both -// settings using the following table (first column contains local values for -// the preferences, first row contains values which came from sync, where _ -// means that the value did not come): -// -// NL* 0_ 1_ _0 _1 00 01 10 11 -// -// 00 00 11 00 11 00 11 11 11 -// 01 00* 11* x x 00 00 00 11 -// 10 00* 00* x x 00 00 00 11 -// 11 00 11 00 11 00 00 00 11 -// -// *these cases only possible on mobile platforms, where we sync only priority -// preference data type. -// -// The service observes changes to both preferences (e.g. changes from sync, -// changes from UI) and propagates the change to the other preference if needed. -// -// Note: componetization of this service currently is impossible, because it -// depends on PrefServiceSyncable https://crbug.com/522536. -class PasswordManagerSettingMigratorService - : public KeyedService, - public sync_preferences::PrefServiceSyncableObserver { - public: - explicit PasswordManagerSettingMigratorService( - sync_preferences::PrefServiceSyncable* prefs); - ~PasswordManagerSettingMigratorService() override; - - void Shutdown() override; - - // PrefServiceSyncableObserver: - void OnIsSyncingChanged() override; - - void InitializeMigration(syncer::SyncService* sync_service); - - // Only use for testing. - static void set_force_disabled_for_testing(bool force_disabled) { - force_disabled_for_testing_ = force_disabled; - } - - private: - // Initializes the observers: preferences observers and sync status observers. - void InitObservers(); - - // Returns true if sync is expected to start for the user. - bool CanSyncStart(); - - // Called when the value of the |kCredentialsEnableService| preference - // changes, and updates the value of |kPasswordManagerSavingEnabled| - // preference accordingly. - void OnCredentialsEnableServicePrefChanged( - const std::string& changed_pref_name); - - // Called when the value of the |kPasswordManagerSavingEnabled| preference - // changes, and updates the value of |kCredentialsEnableService| preference - // accordingly. - void OnPasswordManagerSavingEnabledPrefChanged( - const std::string& changed_pref_name); - - // Determines if model association step was performed. For desktop platforms, - // the condition is that for both priority preferences and regular preferences - // types association step was finished. For mobile platforms, the association - // only for priority prefs is required. - bool WasModelAssociationStepPerformed(); - - // Turns off one pref if another pref is off. - void MigrateOffState(PrefService* prefs); - - // Performs a migration after sync associates models. Migration is performed - // based on initial values for both preferences and values received from - // sync. - void MigrateAfterModelAssociation(PrefService* prefs); - - // Contains values which have come from sync during model association step. - // The vector minimum size is 0 and the vector maximum size is 4: - // * sync_data_ has length equal to 0, when no change to the preferences has - // came from sync. - // * sync_data has length equal to 1, when change to only one preference - // came from sync and another pref already had this value, e.g local state - // 01 and N=1 came from sync. - // * sync_data has length equals to 4, changes to both preference has came - // from sync, e.g. local state is 11 and 01 came from sync. - // This way index 0 corresponds to kCredentialsEnableService, last index - // corresponds to kPasswordManagerSavingEnabled if size of sync_data_ equals - // to 4, otherwise the vector contains the value only for one preference. - std::vector<bool> sync_data_; - - // The initial value for kCredentialsEnableService. - bool initial_new_pref_value_; - // The initial value for kPasswordManagerSavingEnabled. - bool initial_legacy_pref_value_; - - sync_preferences::PrefServiceSyncable* prefs_; - syncer::SyncService* sync_service_; - - PrefChangeRegistrar pref_change_registrar_; - - // If true, the service will refuse to initialize despite Field Trial - // settings. - // Default value is false. Only use for testing. - static bool force_disabled_for_testing_; - - DISALLOW_COPY_AND_ASSIGN(PasswordManagerSettingMigratorService); -}; - -} // namespace password_manager - -#endif // COMPONENTS_PASSWORD_MANAGER_SYNC_BROWSER_PASSWORD_MANAGER_SETTING_MIGRATOR_SERVICE_H_
diff --git a/components/password_manager/sync/browser/password_manager_setting_migrator_service_unittest.cc b/components/password_manager/sync/browser/password_manager_setting_migrator_service_unittest.cc deleted file mode 100644 index 3ba8c19..0000000 --- a/components/password_manager/sync/browser/password_manager_setting_migrator_service_unittest.cc +++ /dev/null
@@ -1,463 +0,0 @@ -// Copyright (c) 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. - -#include <utility> - -#include "base/json/json_writer.h" -#include "base/macros.h" -#include "base/metrics/field_trial.h" -#include "base/test/histogram_tester.h" -#include "base/values.h" -#include "build/build_config.h" -#include "components/password_manager/core/browser/password_manager.h" -#include "components/password_manager/core/common/password_manager_pref_names.h" -#include "components/password_manager/sync/browser/password_manager_setting_migrator_service.h" -#include "components/prefs/pref_service.h" -#include "components/sync/driver/fake_sync_service.h" -#include "components/sync/model/attachments/attachment_service_proxy_for_test.h" -#include "components/sync/model/fake_sync_change_processor.h" -#include "components/sync/model/sync_error_factory.h" -#include "components/sync/model/sync_error_factory_mock.h" -#include "components/sync/protocol/sync.pb.h" -#include "components/sync_preferences/pref_model_associator_client.h" -#include "components/sync_preferences/pref_service_mock_factory.h" -#include "components/sync_preferences/pref_service_syncable.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace { - -const char kFieldTrialName[] = "PasswordManagerSettingsMigration"; -const char kEnabledGroupName[] = "Enable"; -const char kDisabledGroupName[] = "Disable"; - -const char kInitialValuesHistogramName[] = - "PasswordManager.SettingsReconciliation.InitialValues"; - -const char kInitialAndFinalValuesHistogramName[] = - "PasswordManager.SettingsReconciliation.InitialAndFinalValues"; - -enum BooleanPrefState { - OFF, - ON, - EMPTY, // datatype bucket is empty -}; - -// Enum used for histogram tracking of the initial values for the legacy and new -// preferences. -enum PasswordManagerPreferencesInitialValues { - N0L0, - N0L1, - N1L0, - N1L1, - NUM_INITIAL_VALUES, -}; - -// Enum used for histogram tracking of the combined initial values and final -// values for the legacy and new preferences. -enum PasswordManagerPreferencesInitialAndFinalValues { - I00F00, - I00F01, - I00F10, - I00F11, - I01F00, - I01F01, - I01F10, - I01F11, - I10F00, - I10F01, - I10F10, - I10F11, - I11F00, - I11F01, - I11F10, - I11F11, - NUM_INITIAL_AND_FINAL_VALUES, -}; - -syncer::SyncData CreatePrefSyncData(const std::string& name, bool value) { - base::Value bool_value(value); - std::string serialized; - base::JSONWriter::Write(bool_value, &serialized); - sync_pb::EntitySpecifics specifics; - sync_pb::PreferenceSpecifics* pref = nullptr; - if (name == password_manager::prefs::kPasswordManagerSavingEnabled) - pref = specifics.mutable_preference(); - else if (name == password_manager::prefs::kCredentialsEnableService) - pref = specifics.mutable_priority_preference()->mutable_preference(); - else - NOTREACHED() << "Wrong preference name: " << name; - pref->set_name(name); - pref->set_value(serialized); - return syncer::SyncData::CreateRemoteData( - 1, specifics, base::Time(), syncer::AttachmentIdList(), - syncer::AttachmentServiceProxyForTest::Create()); -} - -// Emulates start of the syncing for the specific sync type. If |name| is -// kPasswordManagerSavingEnabled preference, then it's PREFERENCE data type. -// If |name| is kCredentialsEnableService pref, then it's PRIORITY_PREFERENCE -// data type. -void StartSyncingPref(sync_preferences::PrefServiceSyncable* prefs, - const std::string& name, - BooleanPrefState pref_state_in_sync) { - syncer::SyncDataList sync_data_list; - if (pref_state_in_sync == EMPTY) { - sync_data_list = syncer::SyncDataList(); - } else { - sync_data_list.push_back( - CreatePrefSyncData(name, pref_state_in_sync == ON)); - } - - syncer::ModelType type = syncer::UNSPECIFIED; - if (name == password_manager::prefs::kPasswordManagerSavingEnabled) - type = syncer::PREFERENCES; - else if (name == password_manager::prefs::kCredentialsEnableService) - type = syncer::PRIORITY_PREFERENCES; - ASSERT_NE(syncer::UNSPECIFIED, type) << "Wrong preference name: " << name; - syncer::SyncableService* sync = prefs->GetSyncableService(type); - sync->MergeDataAndStartSyncing(type, sync_data_list, - std::unique_ptr<syncer::SyncChangeProcessor>( - new syncer::FakeSyncChangeProcessor), - std::unique_ptr<syncer::SyncErrorFactory>( - new syncer::SyncErrorFactoryMock)); -} - -class SyncServiceMock : public syncer::FakeSyncService { - public: - bool IsFirstSetupComplete() const override { return true; } - - bool CanSyncStart() const override { return can_sync_start_; } - - void SetCanSyncStart(bool can_sync_start) { - can_sync_start_ = can_sync_start; - } - - private: - bool can_sync_start_ = true; -}; - -class TestPrefModelAssociatorClient - : public sync_preferences::PrefModelAssociatorClient { - public: - TestPrefModelAssociatorClient() {} - ~TestPrefModelAssociatorClient() override {} - - // PrefModelAssociatorClient implementation. - bool IsMergeableListPreference(const std::string& pref_name) const override { - return false; - } - - bool IsMergeableDictionaryPreference( - const std::string& pref_name) const override { - return false; - } - - private: - DISALLOW_COPY_AND_ASSIGN(TestPrefModelAssociatorClient); -}; - -} // namespace - -namespace password_manager { - -class PasswordManagerSettingMigratorServiceTest : public testing::Test { - protected: - PasswordManagerSettingMigratorServiceTest() {} - ~PasswordManagerSettingMigratorServiceTest() override {} - - void SetUp() override { - SetupPreferenceMigrationEnvironment(); - EnforcePasswordManagerSettingMigrationExperiment(kEnabledGroupName); - } - - void SetupLocalPrefState(const std::string& name, BooleanPrefState state) { - if (state == ON) - prefs()->SetBoolean(name, true); - else if (state == OFF) - prefs()->SetBoolean(name, false); - else if (state == EMPTY) - ASSERT_TRUE(prefs()->FindPreference(name)->IsDefaultValue()); - } - - sync_preferences::PrefServiceSyncable* prefs() { return pref_service_.get(); } - - void SetupPreferenceMigrationEnvironment() { - sync_preferences::PrefServiceMockFactory factory; - factory.SetPrefModelAssociatorClient(&client_); - scoped_refptr<user_prefs::PrefRegistrySyncable> pref_registry( - new user_prefs::PrefRegistrySyncable); - password_manager::PasswordManager::RegisterProfilePrefs( - pref_registry.get()); - std::unique_ptr<sync_preferences::PrefServiceSyncable> - pref_service_syncable = factory.CreateSyncable(pref_registry.get()); - migration_service_.reset( - new PasswordManagerSettingMigratorService(pref_service_syncable.get())); - pref_service_ = std::move(pref_service_syncable); - } - - void ExpectValuesForBothPrefValues(bool new_pref_value, bool old_pref_value) { - EXPECT_EQ(new_pref_value, - prefs()->GetBoolean(prefs::kCredentialsEnableService)); - EXPECT_EQ(old_pref_value, - prefs()->GetBoolean(prefs::kPasswordManagerSavingEnabled)); - } - - SyncServiceMock* profile_sync_service() { return &sync_service_; } - - void NotifyProfileAdded() { - migration_service_->InitializeMigration(&sync_service_); - } - - void EnforcePasswordManagerSettingMigrationExperiment(const char* name) { - // The existing instance of FieldTrialList should be deleted before creating - // new one, so reset() is called in order to do so. - field_trial_list_.reset(); - field_trial_list_.reset(new base::FieldTrialList(nullptr)); - base::FieldTrialList::CreateFieldTrial(kFieldTrialName, name); - } - - private: - std::unique_ptr<base::FieldTrialList> field_trial_list_; - TestPrefModelAssociatorClient client_; - SyncServiceMock sync_service_; - std::unique_ptr<sync_preferences::PrefServiceSyncable> pref_service_; - std::unique_ptr<PasswordManagerSettingMigratorService> migration_service_; - - DISALLOW_COPY_AND_ASSIGN(PasswordManagerSettingMigratorServiceTest); -}; - -TEST_F(PasswordManagerSettingMigratorServiceTest, TestMigrationOnLocalChanges) { - const struct { - const char* group; - const char* pref_name; - bool pref_value; - bool expected_new_pref_value; - bool expected_old_pref_value; - } kTestingTable[] = { - {kEnabledGroupName, prefs::kPasswordManagerSavingEnabled, true, true, - true}, - {kEnabledGroupName, prefs::kPasswordManagerSavingEnabled, false, false, - false}, - {kEnabledGroupName, prefs::kCredentialsEnableService, true, true, true}, - {kEnabledGroupName, prefs::kCredentialsEnableService, false, false, - false}, - {kDisabledGroupName, prefs::kPasswordManagerSavingEnabled, false, true, - false}, - {kDisabledGroupName, prefs::kCredentialsEnableService, false, false, - true}}; - - for (const auto& test_case : kTestingTable) { - SetupPreferenceMigrationEnvironment(); - EnforcePasswordManagerSettingMigrationExperiment(test_case.group); - prefs()->SetBoolean(prefs::kCredentialsEnableService, - !test_case.pref_value); - prefs()->SetBoolean(prefs::kPasswordManagerSavingEnabled, - !test_case.pref_value); - NotifyProfileAdded(); - base::HistogramTester tester; - prefs()->SetBoolean(test_case.pref_name, test_case.pref_value); - ExpectValuesForBothPrefValues(test_case.expected_new_pref_value, - test_case.expected_old_pref_value); - EXPECT_THAT(tester.GetAllSamples(kInitialValuesHistogramName), - testing::IsEmpty()); - } -} - -TEST_F(PasswordManagerSettingMigratorServiceTest, - ReconcileWhenWhenBothPrefsTypesArrivesFromSync) { - const struct { - BooleanPrefState new_pref_local_value; - BooleanPrefState old_pref_local_value; - BooleanPrefState new_pref_sync_value; - BooleanPrefState old_pref_sync_value; - bool result_value; - PasswordManagerPreferencesInitialValues histogram_initial_value; - PasswordManagerPreferencesInitialAndFinalValues histogram_initial_and_final; - } kTestingTable[] = { -#if defined(OS_ANDROID) || defined(OS_IOS) - {ON, OFF, ON, EMPTY, false, N1L0, I10F00}, - {ON, OFF, OFF, EMPTY, false, N1L0, I10F00}, - {ON, OFF, EMPTY, EMPTY, false, N1L0, I10F00}, - {ON, ON, ON, EMPTY, true, N1L1, I11F11}, - {ON, ON, OFF, EMPTY, false, N1L1, I11F00}, - {OFF, OFF, ON, EMPTY, true, N0L0, I00F11}, - {OFF, OFF, OFF, EMPTY, false, N0L0, I00F00}, - {OFF, ON, ON, EMPTY, true, N0L1, I01F11}, - {OFF, ON, OFF, EMPTY, false, N0L1, I01F00}, - {OFF, ON, EMPTY, EMPTY, false, N0L1, I01F00}, -#else - {EMPTY, EMPTY, EMPTY, EMPTY, true, N1L1, I11F11}, - {EMPTY, EMPTY, EMPTY, OFF, false, N1L1, I11F00}, - {EMPTY, EMPTY, EMPTY, ON, true, N1L1, I11F11}, - {EMPTY, EMPTY, OFF, EMPTY, false, N1L1, I11F00}, - {EMPTY, EMPTY, ON, EMPTY, true, N1L1, I11F11}, - {OFF, OFF, EMPTY, EMPTY, false, N0L0, I00F00}, - {OFF, OFF, OFF, OFF, false, N0L0, I00F00}, - {OFF, OFF, OFF, ON, true, N0L0, I00F11}, - {OFF, OFF, ON, OFF, true, N0L0, I00F11}, - {OFF, ON, OFF, ON, false, N0L1, I01F00}, - {OFF, ON, ON, OFF, false, N0L1, I01F00}, - {OFF, ON, ON, ON, true, N0L1, I01F11}, - {ON, OFF, EMPTY, EMPTY, false, N1L0, I10F00}, - {ON, OFF, OFF, ON, false, N1L0, I10F00}, - {ON, OFF, ON, OFF, false, N1L0, I10F00}, - {ON, OFF, ON, ON, true, N1L0, I10F11}, - {ON, ON, EMPTY, OFF, false, N1L1, I11F00}, - {ON, ON, EMPTY, ON, true, N1L1, I11F11}, - {ON, ON, OFF, EMPTY, false, N1L1, I11F00}, - {ON, ON, OFF, OFF, false, N1L1, I11F00}, - {ON, ON, OFF, ON, false, N1L1, I11F00}, - {ON, ON, ON, EMPTY, true, N1L1, I11F11}, - {ON, ON, ON, OFF, false, N1L1, I11F00}, - {ON, ON, ON, ON, true, N1L1, I11F11}, -#endif - }; - - for (const auto& test_case : kTestingTable) { - SetupPreferenceMigrationEnvironment(); - EnforcePasswordManagerSettingMigrationExperiment(kEnabledGroupName); - SCOPED_TRACE(testing::Message("Local data = ") - << test_case.new_pref_local_value << " " - << test_case.old_pref_local_value); - SCOPED_TRACE(testing::Message("Sync data = ") - << test_case.new_pref_sync_value << " " - << test_case.old_pref_sync_value); - SetupLocalPrefState(prefs::kPasswordManagerSavingEnabled, - test_case.old_pref_local_value); - SetupLocalPrefState(prefs::kCredentialsEnableService, - test_case.new_pref_local_value); - base::HistogramTester tester; - NotifyProfileAdded(); - StartSyncingPref(prefs(), prefs::kCredentialsEnableService, - test_case.new_pref_sync_value); -#if !defined(OS_ANDROID) && !defined(OS_IOS) - StartSyncingPref(prefs(), prefs::kPasswordManagerSavingEnabled, - test_case.old_pref_sync_value); -#endif - ExpectValuesForBothPrefValues(test_case.result_value, - test_case.result_value); - EXPECT_THAT(tester.GetAllSamples(kInitialValuesHistogramName), - testing::ElementsAre( - base::Bucket(test_case.histogram_initial_value, 1))); - EXPECT_THAT(tester.GetAllSamples(kInitialAndFinalValuesHistogramName), - testing::ElementsAre( - base::Bucket(test_case.histogram_initial_and_final, 1))); - } -} - -TEST_F(PasswordManagerSettingMigratorServiceTest, - DoNotReconcileWhenWhenBothPrefsTypesArrivesFromSync) { - const struct { - BooleanPrefState new_pref_local_value; - BooleanPrefState old_pref_local_value; - BooleanPrefState new_pref_sync_value; - BooleanPrefState old_pref_sync_value; - bool result_new_pref_value; - bool result_old_pref_value; - PasswordManagerPreferencesInitialValues histogram_initial_value; - } kTestingTable[] = { -#if defined(OS_ANDROID) || defined(OS_IOS) - {ON, OFF, ON, EMPTY, true, false, N1L0}, - {ON, OFF, OFF, EMPTY, false, false, N1L0}, - {ON, OFF, EMPTY, EMPTY, true, false, N1L0}, - {ON, ON, ON, EMPTY, true, true, N1L1}, - {ON, ON, OFF, EMPTY, false, true, N1L1}, - {OFF, OFF, ON, EMPTY, true, false, N0L0}, - {OFF, OFF, OFF, EMPTY, false, false, N0L0}, - {OFF, ON, ON, EMPTY, true, true, N0L1}, - {OFF, ON, OFF, EMPTY, false, true, N0L1}, - {OFF, ON, EMPTY, EMPTY, false, true, N0L1}, -#else - {OFF, OFF, OFF, ON, false, true, N0L0}, - {OFF, OFF, ON, OFF, true, false, N0L0}, - {OFF, OFF, ON, ON, true, true, N0L0}, - {OFF, ON, EMPTY, OFF, false, false, N0L1}, - {OFF, ON, EMPTY, ON, false, true, N0L1}, - {OFF, ON, OFF, EMPTY, false, true, N0L1}, - {OFF, ON, OFF, OFF, false, false, N0L1}, - {OFF, ON, OFF, ON, false, true, N0L1}, - {OFF, ON, ON, EMPTY, true, true, N0L1}, - {OFF, ON, ON, OFF, true, false, N0L1}, - {OFF, ON, ON, ON, true, true, N0L1}, - {ON, OFF, OFF, ON, false, true, N1L0}, - {ON, OFF, ON, OFF, true, false, N1L0}, - {ON, OFF, ON, ON, true, true, N1L0}, - {ON, ON, EMPTY, OFF, true, false, N1L1}, - {ON, ON, EMPTY, ON, true, true, N1L1}, - {ON, ON, OFF, EMPTY, false, true, N1L1}, - {ON, ON, OFF, OFF, false, false, N1L1}, - {ON, ON, OFF, ON, false, true, N1L1}, - {ON, ON, ON, EMPTY, true, true, N1L1}, - {ON, ON, ON, OFF, true, false, N1L1}, - {ON, ON, ON, ON, true, true, N1L1}, -#endif - }; - - for (const auto& test_case : kTestingTable) { - SetupPreferenceMigrationEnvironment(); - EnforcePasswordManagerSettingMigrationExperiment(kDisabledGroupName); - SCOPED_TRACE(testing::Message("Local data = ") - << test_case.new_pref_local_value << " " - << test_case.old_pref_local_value); - SCOPED_TRACE(testing::Message("Sync data = ") - << test_case.new_pref_sync_value << " " - << test_case.old_pref_sync_value); - SetupLocalPrefState(prefs::kPasswordManagerSavingEnabled, - test_case.old_pref_local_value); - SetupLocalPrefState(prefs::kCredentialsEnableService, - test_case.new_pref_local_value); - base::HistogramTester tester; - NotifyProfileAdded(); - StartSyncingPref(prefs(), prefs::kCredentialsEnableService, - test_case.new_pref_sync_value); -#if !defined(OS_ANDROID) && !defined(OS_IOS) - StartSyncingPref(prefs(), prefs::kPasswordManagerSavingEnabled, - test_case.old_pref_sync_value); -#endif - ExpectValuesForBothPrefValues(test_case.result_new_pref_value, - test_case.result_old_pref_value); - EXPECT_THAT(tester.GetAllSamples(kInitialValuesHistogramName), - testing::ElementsAre( - base::Bucket(test_case.histogram_initial_value, 1))); - EXPECT_THAT(tester.GetAllSamples(kInitialAndFinalValuesHistogramName), - testing::IsEmpty()); - } -} - -TEST_F(PasswordManagerSettingMigratorServiceTest, - ReconcileWhenSyncIsNotExpectedPasswordManagerEnabledOff) { - prefs()->SetBoolean(prefs::kPasswordManagerSavingEnabled, false); - profile_sync_service()->SetCanSyncStart(false); - base::HistogramTester tester; - NotifyProfileAdded(); - ExpectValuesForBothPrefValues(false, false); - EXPECT_THAT(tester.GetAllSamples(kInitialAndFinalValuesHistogramName), - testing::ElementsAre(base::Bucket(I10F00, 1))); -} - -TEST_F(PasswordManagerSettingMigratorServiceTest, - ReconcileWhenSyncIsNotExpectedPasswordManagerEnabledOn) { - prefs()->SetBoolean(prefs::kPasswordManagerSavingEnabled, true); - ASSERT_EQ(prefs()->GetBoolean(prefs::kCredentialsEnableService), true); - profile_sync_service()->SetCanSyncStart(false); - base::HistogramTester tester; - NotifyProfileAdded(); - ExpectValuesForBothPrefValues(true, true); - EXPECT_THAT(tester.GetAllSamples(kInitialAndFinalValuesHistogramName), - testing::ElementsAre(base::Bucket(I11F11, 1))); -} - -TEST_F(PasswordManagerSettingMigratorServiceTest, - ReconcileWhenSyncIsNotExpectedDefaultValuesForPrefs) { - ASSERT_EQ(prefs()->GetBoolean(prefs::kCredentialsEnableService), true); - profile_sync_service()->SetCanSyncStart(false); - base::HistogramTester tester; - NotifyProfileAdded(); - ExpectValuesForBothPrefValues(true, true); - EXPECT_THAT(tester.GetAllSamples(kInitialAndFinalValuesHistogramName), - testing::ElementsAre(base::Bucket(I11F11, 1))); -} - -} // namespace password_manager
diff --git a/components/prefs/pref_value_store.cc b/components/prefs/pref_value_store.cc index 4c7a4e5..da58b23 100644 --- a/components/prefs/pref_value_store.cc +++ b/components/prefs/pref_value_store.cc
@@ -55,9 +55,11 @@ PrefStore* user_prefs, PrefStore* recommended_prefs, PrefStore* default_prefs, - PrefNotifier* pref_notifier) + PrefNotifier* pref_notifier, + std::unique_ptr<Delegate> delegate) : pref_notifier_(pref_notifier), - initialization_failed_(false) { + initialization_failed_(false), + delegate_(std::move(delegate)) { InitPrefStore(MANAGED_STORE, managed_prefs); InitPrefStore(SUPERVISED_USER_STORE, supervised_user_prefs); InitPrefStore(EXTENSION_STORE, extension_prefs); @@ -67,6 +69,11 @@ InitPrefStore(DEFAULT_STORE, default_prefs); CheckInitializationCompleted(); + if (delegate_) { + delegate_->Init(managed_prefs, supervised_user_prefs, extension_prefs, + command_line_prefs, user_prefs, recommended_prefs, + default_prefs, pref_notifier); + } } PrefValueStore::~PrefValueStore() {} @@ -79,7 +86,8 @@ PrefStore* user_prefs, PrefStore* recommended_prefs, PrefStore* default_prefs, - PrefNotifier* pref_notifier) { + PrefNotifier* pref_notifier, + std::unique_ptr<Delegate> delegate) { DCHECK(pref_notifier); if (!managed_prefs) managed_prefs = GetPrefStore(MANAGED_STORE); @@ -96,9 +104,10 @@ if (!default_prefs) default_prefs = GetPrefStore(DEFAULT_STORE); - return new PrefValueStore( - managed_prefs, supervised_user_prefs, extension_prefs, command_line_prefs, - user_prefs, recommended_prefs, default_prefs, pref_notifier); + return new PrefValueStore(managed_prefs, supervised_user_prefs, + extension_prefs, command_line_prefs, user_prefs, + recommended_prefs, default_prefs, pref_notifier, + std::move(delegate)); } void PrefValueStore::set_callback(const PrefChangedCallback& callback) { @@ -185,6 +194,8 @@ void PrefValueStore::UpdateCommandLinePrefStore(PrefStore* command_line_prefs) { InitPrefStore(COMMAND_LINE_STORE, command_line_prefs); + if (delegate_) + delegate_->UpdateCommandLinePrefStore(command_line_prefs); } bool PrefValueStore::PrefValueInStore(
diff --git a/components/prefs/pref_value_store.h b/components/prefs/pref_value_store.h index eeab306..7fb84f0 100644 --- a/components/prefs/pref_value_store.h +++ b/components/prefs/pref_value_store.h
@@ -31,6 +31,27 @@ public: typedef base::Callback<void(const std::string&)> PrefChangedCallback; + // Delegate used to observe certain events in the |PrefValueStore|'s lifetime. + class Delegate { + public: + virtual ~Delegate() {} + + // Called by the PrefValueStore constructor with the PrefStores passed to + // it. + virtual void Init(PrefStore* managed_prefs, + PrefStore* supervised_user_prefs, + PrefStore* extension_prefs, + PrefStore* command_line_prefs, + PrefStore* user_prefs, + PrefStore* recommended_prefs, + PrefStore* default_prefs, + PrefNotifier* pref_notifier) = 0; + + // Called whenever PrefValueStore::UpdateCommandLinePrefStore is called, + // with the same argument. + virtual void UpdateCommandLinePrefStore(PrefStore* command_line_prefs) = 0; + }; + // PrefStores must be listed here in order from highest to lowest priority. // MANAGED contains all managed preference values that are provided by // mandatory policies (e.g. Windows Group Policy or cloud policy). @@ -77,19 +98,24 @@ PrefStore* user_prefs, PrefStore* recommended_prefs, PrefStore* default_prefs, - PrefNotifier* pref_notifier); + PrefNotifier* pref_notifier, + std::unique_ptr<Delegate> delegate = nullptr); virtual ~PrefValueStore(); // Creates a clone of this PrefValueStore with PrefStores overwritten // by the parameters passed, if unequal NULL. - PrefValueStore* CloneAndSpecialize(PrefStore* managed_prefs, - PrefStore* supervised_user_prefs, - PrefStore* extension_prefs, - PrefStore* command_line_prefs, - PrefStore* user_prefs, - PrefStore* recommended_prefs, - PrefStore* default_prefs, - PrefNotifier* pref_notifier); + // + // The new PrefValueStore is passed the |delegate| in its constructor. + PrefValueStore* CloneAndSpecialize( + PrefStore* managed_prefs, + PrefStore* supervised_user_prefs, + PrefStore* extension_prefs, + PrefStore* command_line_prefs, + PrefStore* user_prefs, + PrefStore* recommended_prefs, + PrefStore* default_prefs, + PrefNotifier* pref_notifier, + std::unique_ptr<Delegate> delegate = nullptr); // A PrefValueStore can have exactly one callback that is directly // notified of preferences changing in the store. This does not @@ -255,6 +281,9 @@ // True if not all of the PrefStores were initialized successfully. bool initialization_failed_; + // Might be null. + std::unique_ptr<Delegate> delegate_; + DISALLOW_COPY_AND_ASSIGN(PrefValueStore); };
diff --git a/components/signin/core/browser/signin_header_helper.cc b/components/signin/core/browser/signin_header_helper.cc index cd2c779..0331f65 100644 --- a/components/signin/core/browser/signin_header_helper.cc +++ b/components/signin/core/browser/signin_header_helper.cc
@@ -33,10 +33,6 @@ is_saml(false), continue_url(""), is_same_tab(false) { -#if !defined(OS_IOS) - child_id = 0; - route_id = 0; -#endif // !defined(OS_IOS) } ManageAccountsParams::ManageAccountsParams(const ManageAccountsParams& other) =
diff --git a/components/signin/core/browser/signin_header_helper.h b/components/signin/core/browser/signin_header_helper.h index 7975dad..c7249b2 100644 --- a/components/signin/core/browser/signin_header_helper.h +++ b/components/signin/core/browser/signin_header_helper.h
@@ -67,14 +67,6 @@ // Whether the continue URL should be loaded in the same tab. bool is_same_tab; -// iOS has no notion of route and child IDs. -#if !defined(OS_IOS) - // The child ID associated with the web content of the request. - int child_id; - // The route ID associated with the web content of the request. - int route_id; -#endif // !defined(OS_IOS) - ManageAccountsParams(); ManageAccountsParams(const ManageAccountsParams& other); };
diff --git a/components/sync_preferences/pref_service_syncable.cc b/components/sync_preferences/pref_service_syncable.cc index ac32f54..488f859 100644 --- a/components/sync_preferences/pref_service_syncable.cc +++ b/components/sync_preferences/pref_service_syncable.cc
@@ -14,12 +14,35 @@ #include "components/prefs/overlay_user_pref_store.h" #include "components/prefs/pref_notifier_impl.h" #include "components/prefs/pref_registry.h" -#include "components/prefs/pref_value_store.h" #include "components/sync_preferences/pref_model_associator.h" #include "components/sync_preferences/pref_service_syncable_observer.h" +#include "services/preferences/public/cpp/persistent_pref_store_client.h" +#include "services/preferences/public/cpp/pref_registry_serializer.h" +#include "services/preferences/public/cpp/pref_store_impl.h" +#include "services/preferences/public/cpp/registering_delegate.h" +#include "services/preferences/public/interfaces/preferences.mojom.h" +#include "services/service_manager/public/cpp/connector.h" namespace sync_preferences { +namespace { + +// This only needs to be called once per incognito profile. +void InitIncognitoService(service_manager::Connector* incognito_connector, + service_manager::Connector* user_connector) { + prefs::mojom::PrefStoreConnectorPtr connector; + user_connector->BindInterface(prefs::mojom::kServiceName, &connector); + auto config = prefs::mojom::PersistentPrefStoreConfiguration::New(); + config->set_incognito_configuration( + prefs::mojom::IncognitoPersistentPrefStoreConfiguration::New( + std::move(connector))); + prefs::mojom::PrefServiceControlPtr control; + incognito_connector->BindInterface(prefs::mojom::kServiceName, &control); + control->Init(std::move(config)); +} + +} // namespace + PrefServiceSyncable::PrefServiceSyncable( PrefNotifierImpl* pref_notifier, PrefValueStore* pref_value_store, @@ -70,28 +93,45 @@ PrefServiceSyncable* PrefServiceSyncable::CreateIncognitoPrefService( PrefStore* incognito_extension_pref_store, - const std::vector<const char*>& overlay_pref_names) { + const std::vector<const char*>& overlay_pref_names, + std::set<PrefValueStore::PrefStoreType> already_connected_types, + service_manager::Connector* incognito_connector, + service_manager::Connector* user_connector) { pref_service_forked_ = true; PrefNotifierImpl* pref_notifier = new PrefNotifierImpl(); - OverlayUserPrefStore* incognito_pref_store = - new OverlayUserPrefStore(user_pref_store_.get()); - for (const char* overlay_pref_name : overlay_pref_names) - incognito_pref_store->RegisterOverlayPref(overlay_pref_name); scoped_refptr<user_prefs::PrefRegistrySyncable> forked_registry = static_cast<user_prefs::PrefRegistrySyncable*>(pref_registry_.get()) ->ForkForIncognito(); + + scoped_refptr<OverlayUserPrefStore> incognito_pref_store; + std::unique_ptr<RegisteringDelegate> delegate; + if (incognito_connector) { + DCHECK(user_connector); + incognito_pref_store = CreateOverlayUsingPrefService( + forked_registry.get(), std::move(already_connected_types), + incognito_connector, user_connector); + prefs::mojom::PrefStoreRegistryPtr registry; + incognito_connector->BindInterface(prefs::mojom::kServiceName, ®istry); + delegate = base::MakeUnique<RegisteringDelegate>(std::move(registry)); + } else { + incognito_pref_store = new OverlayUserPrefStore(user_pref_store_.get()); + } + + for (const char* overlay_pref_name : overlay_pref_names) + incognito_pref_store->RegisterOverlayPref(overlay_pref_name); + PrefServiceSyncable* incognito_service = new PrefServiceSyncable( pref_notifier, pref_value_store_->CloneAndSpecialize(NULL, // managed NULL, // supervised_user incognito_extension_pref_store, NULL, // command_line_prefs - incognito_pref_store, + incognito_pref_store.get(), NULL, // recommended forked_registry->defaults().get(), - pref_notifier), - incognito_pref_store, forked_registry.get(), + pref_notifier, std::move(delegate)), + incognito_pref_store.get(), forked_registry.get(), pref_sync_associator_.client(), read_error_callback_, false); return incognito_service; } @@ -186,4 +226,35 @@ priority_pref_sync_associator_.ProcessPrefChange(name); } +OverlayUserPrefStore* PrefServiceSyncable::CreateOverlayUsingPrefService( + user_prefs::PrefRegistrySyncable* pref_registry, + std::set<PrefValueStore::PrefStoreType> already_connected_types, + service_manager::Connector* incognito_connector, + service_manager::Connector* user_connector) const { + InitIncognitoService(incognito_connector, user_connector); + + prefs::mojom::PrefStoreConnectorPtr pref_connector; + incognito_connector->BindInterface(prefs::mojom::kServiceName, + &pref_connector); + std::vector<PrefValueStore::PrefStoreType> in_process_types( + already_connected_types.begin(), already_connected_types.end()); + + // Connect to the writable, in-memory incognito pref store. + prefs::mojom::PersistentPrefStoreConnectionPtr connection; + prefs::mojom::PersistentPrefStoreConnectionPtr incognito_connection; + std::vector<prefs::mojom::PrefRegistrationPtr> remote_defaults; + std::unordered_map<PrefValueStore::PrefStoreType, + prefs::mojom::PrefStoreConnectionPtr> + other_pref_stores; + mojo::SyncCallRestrictions::ScopedAllowSyncCall allow_sync_calls; + bool success = pref_connector->Connect( + prefs::SerializePrefRegistry(*pref_registry), in_process_types, + &incognito_connection, &connection, &remote_defaults, &other_pref_stores); + DCHECK(success); + + return new OverlayUserPrefStore( + new prefs::PersistentPrefStoreClient(std::move(incognito_connection)), + user_pref_store_.get()); +} + } // namespace sync_preferences
diff --git a/components/sync_preferences/pref_service_syncable.h b/components/sync_preferences/pref_service_syncable.h index 7fc44d0..2af6803 100644 --- a/components/sync_preferences/pref_service_syncable.h +++ b/components/sync_preferences/pref_service_syncable.h
@@ -14,9 +14,16 @@ #include "base/macros.h" #include "components/pref_registry/pref_registry_syncable.h" #include "components/prefs/pref_service.h" +#include "components/prefs/pref_value_store.h" #include "components/sync_preferences/pref_model_associator.h" #include "components/sync_preferences/synced_pref_observer.h" +class OverlayUserPrefStore; + +namespace service_manager { +class Connector; +} + namespace syncer { class SyncableService; } @@ -51,7 +58,10 @@ // whose changes will not be persisted by the returned incognito pref service. PrefServiceSyncable* CreateIncognitoPrefService( PrefStore* incognito_extension_pref_store, - const std::vector<const char*>& overlay_pref_names); + const std::vector<const char*>& overlay_pref_names, + std::set<PrefValueStore::PrefStoreType> already_connected_types, + service_manager::Connector* incognito_connector, + service_manager::Connector* user_connector); // Returns true if preferences state has synchronized with the remote // preferences. If true is returned it can be assumed the local preferences @@ -105,6 +115,14 @@ // sent to the syncer. void ProcessPrefChange(const std::string& name); + // Create an |OverlayUserPrefStore| where the overlayed in-memory pref store + // is accessed remotely through the pref service. + OverlayUserPrefStore* CreateOverlayUsingPrefService( + user_prefs::PrefRegistrySyncable* pref_registry, + std::set<PrefValueStore::PrefStoreType> already_connected_types, + service_manager::Connector* incognito_connector, + service_manager::Connector* user_connector) const; + // Whether CreateIncognitoPrefService() has been called to create a // "forked" PrefService. bool pref_service_forked_;
diff --git a/components/sync_preferences/pref_service_syncable_factory.cc b/components/sync_preferences/pref_service_syncable_factory.cc index 66d98fe..bf3367b9 100644 --- a/components/sync_preferences/pref_service_syncable_factory.cc +++ b/components/sync_preferences/pref_service_syncable_factory.cc
@@ -11,7 +11,7 @@ #include "components/prefs/pref_notifier_impl.h" #include "components/prefs/pref_value_store.h" #include "components/sync_preferences/pref_service_syncable.h" -#include "services/preferences/public/cpp/pref_store_adapter.h" +#include "services/preferences/public/cpp/registering_delegate.h" #include "services/preferences/public/interfaces/preferences.mojom.h" #include "services/service_manager/public/cpp/connector.h" @@ -55,54 +55,26 @@ pref_model_associator_client_ = pref_model_associator_client; } -namespace { - -// Expose the |backing_pref_store| through the prefs service. -scoped_refptr<::PrefStore> CreateRegisteredPrefStore( - prefs::mojom::PrefStoreRegistry* registry, - scoped_refptr<::PrefStore> backing_pref_store, - PrefValueStore::PrefStoreType type) { - // If we're testing or if the prefs service feature flag is off we don't - // register. - if (!registry || !backing_pref_store) - return backing_pref_store; - - return make_scoped_refptr(new prefs::PrefStoreAdapter( - backing_pref_store, - prefs::PrefStoreImpl::Create(registry, backing_pref_store, type))); -} - -} // namespace - std::unique_ptr<PrefServiceSyncable> PrefServiceSyncableFactory::CreateSyncable( user_prefs::PrefRegistrySyncable* pref_registry, service_manager::Connector* connector) { TRACE_EVENT0("browser", "PrefServiceSyncableFactory::CreateSyncable"); PrefNotifierImpl* pref_notifier = new PrefNotifierImpl(); - prefs::mojom::PrefStoreRegistryPtr registry; - if (connector) + std::unique_ptr<RegisteringDelegate> delegate; + if (connector) { + prefs::mojom::PrefStoreRegistryPtr registry; connector->BindInterface(prefs::mojom::kServiceName, ®istry); - - // Expose all read-only stores through the prefs service. - auto managed = CreateRegisteredPrefStore(registry.get(), managed_prefs_, - PrefValueStore::MANAGED_STORE); - auto supervised = - CreateRegisteredPrefStore(registry.get(), supervised_user_prefs_, - PrefValueStore::SUPERVISED_USER_STORE); - auto extension = CreateRegisteredPrefStore(registry.get(), extension_prefs_, - PrefValueStore::EXTENSION_STORE); - auto command_line = CreateRegisteredPrefStore( - registry.get(), command_line_prefs_, PrefValueStore::COMMAND_LINE_STORE); - auto recommended = CreateRegisteredPrefStore( - registry.get(), recommended_prefs_, PrefValueStore::RECOMMENDED_STORE); + delegate = base::MakeUnique<RegisteringDelegate>(std::move(registry)); + } std::unique_ptr<PrefServiceSyncable> pref_service(new PrefServiceSyncable( pref_notifier, - new PrefValueStore(managed.get(), supervised.get(), extension.get(), - command_line.get(), user_prefs_.get(), - recommended.get(), pref_registry->defaults().get(), - pref_notifier), + new PrefValueStore(managed_prefs_.get(), supervised_user_prefs_.get(), + extension_prefs_.get(), command_line_prefs_.get(), + user_prefs_.get(), recommended_prefs_.get(), + pref_registry->defaults().get(), pref_notifier, + std::move(delegate)), user_prefs_.get(), pref_registry, pref_model_associator_client_, read_error_callback_, async_)); return pref_service;
diff --git a/headless/lib/browser/devtools_api/devtools_connection.js b/headless/lib/browser/devtools_api/devtools_connection.js index b526908..d31d4add3 100644 --- a/headless/lib/browser/devtools_api/devtools_connection.js +++ b/headless/lib/browser/devtools_api/devtools_connection.js
@@ -31,7 +31,7 @@ /** * An object containing pending DevTools protocol commands keyed by id. * - * @private {!Map<number, !Connection.CallbackFunction>} + * @private {!Map<number, !Connection.PendingCommand>} */ this.pendingCommands_ = new Map(); @@ -42,7 +42,7 @@ * An object containing DevTools protocol events we are listening for keyed * by name. * - * @private {!Map<string, !Map<number, !Connection.CallbackFunction>>} + * @private {!Map<string, !Map<number, !Connection.EventFunction>>} */ this.eventListeners_ = new Map(); @@ -62,7 +62,7 @@ * * @param {string} eventName Name of the DevTools protocol event to listen * for. - * @param {!Connection.CallbackFunction} listener The callback issued when we + * @param {!Connection.EventFunction} listener The callback issued when we * receive a DevTools protocol event corresponding to the given name. * @return {number} The id of this event listener. */ @@ -114,7 +114,7 @@ this.transport_.send( JSON.stringify({'method': method, 'id': id, 'params': params})); return new Promise((resolve, reject) => { - this.pendingCommands_.set(id, resolve); + this.pendingCommands_.set(id, {resolve: resolve, reject: reject}); }); } @@ -132,8 +132,9 @@ if (!this.pendingCommands_.has(message.id)) throw new Error('Unrecognized id:' + jsonMessage); if (message.hasOwnProperty('error')) - throw new Error('DevTools protocol error: ' + message.error); - this.pendingCommands_.get(message.id)(message.result); + this.pendingCommands_.get(message.id).reject(message.error); + else + this.pendingCommands_.get(message.id).resolve(message.result); this.pendingCommands_.delete(message.id); } else { if (!message.hasOwnProperty('method') || @@ -164,6 +165,14 @@ /** * @typedef {function(Object): undefined|function(string): undefined} */ -Connection.CallbackFunction; +Connection.EventFunction; + +/** + * @typedef {{ + * resolve: function(!Object), + * reject: function(!Object) + * }} + */ +Connection.PendingCommand; exports = Connection;
diff --git a/ios/chrome/browser/browser_state/browser_state_keyed_service_factories.mm b/ios/chrome/browser/browser_state/browser_state_keyed_service_factories.mm index b52853c4..4bfa06a 100644 --- a/ios/chrome/browser/browser_state/browser_state_keyed_service_factories.mm +++ b/ios/chrome/browser/browser_state/browser_state_keyed_service_factories.mm
@@ -24,7 +24,6 @@ #include "ios/chrome/browser/history/web_history_service_factory.h" #include "ios/chrome/browser/invalidation/ios_chrome_profile_invalidation_provider_factory.h" #include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h" -#include "ios/chrome/browser/passwords/ios_chrome_password_manager_setting_migrator_service_factory.h" #include "ios/chrome/browser/passwords/ios_chrome_password_store_factory.h" #include "ios/chrome/browser/reading_list/reading_list_model_factory.h" #include "ios/chrome/browser/search_engines/template_url_service_factory.h" @@ -97,7 +96,6 @@ IOSChromeLargeIconServiceFactory::GetInstance(); IOSChromeFaviconLoaderFactory::GetInstance(); IOSChromeContentSuggestionsServiceFactory::GetInstance(); - IOSChromePasswordManagerSettingMigratorServiceFactory::GetInstance(); IOSChromePasswordStoreFactory::GetInstance(); IOSChromeProfileInvalidationProviderFactory::GetInstance(); IOSChromeProfileSyncServiceFactory::GetInstance();
diff --git a/ios/chrome/browser/browser_state/chrome_browser_state_manager_impl.cc b/ios/chrome/browser/browser_state/chrome_browser_state_manager_impl.cc index 16aa521f..57fe497a 100644 --- a/ios/chrome/browser/browser_state/chrome_browser_state_manager_impl.cc +++ b/ios/chrome/browser/browser_state/chrome_browser_state_manager_impl.cc
@@ -15,7 +15,6 @@ #include "components/browser_sync/profile_sync_service.h" #include "components/invalidation/impl/profile_invalidation_provider.h" #include "components/password_manager/core/browser/password_store.h" -#include "components/password_manager/sync/browser/password_manager_setting_migrator_service.h" #include "components/prefs/pref_service.h" #include "components/signin/core/browser/account_fetcher_service.h" #include "components/signin/core/browser/account_tracker_service.h" @@ -30,7 +29,6 @@ #include "ios/chrome/browser/chrome_paths.h" #include "ios/chrome/browser/desktop_promotion/desktop_promotion_sync_service_factory.h" #include "ios/chrome/browser/invalidation/ios_chrome_profile_invalidation_provider_factory.h" -#include "ios/chrome/browser/passwords/ios_chrome_password_manager_setting_migrator_service_factory.h" #include "ios/chrome/browser/pref_names.h" #include "ios/chrome/browser/signin/account_consistency_service_factory.h" #include "ios/chrome/browser/signin/account_fetcher_service_factory.h" @@ -215,17 +213,6 @@ ->SetupInvalidationsOnProfileLoad(invalidation_service); ios::AccountReconcilorFactory::GetForBrowserState(browser_state); DesktopPromotionSyncServiceFactory::GetForBrowserState(browser_state); - - // This service is responsible for migration of the legacy password manager - // preference which controls behaviour of Chrome to the new preference which - // controls password management behaviour on Chrome and Android. After - // migration will be performed for all users it's planned to remove the - // migration code, rough time estimates are Q1 2016. - IOSChromePasswordManagerSettingMigratorServiceFactory::GetForBrowserState( - browser_state) - ->InitializeMigration( - IOSChromeProfileSyncServiceFactory::GetForBrowserState( - browser_state)); } void ChromeBrowserStateManagerImpl::AddBrowserStateToCache(
diff --git a/ios/chrome/browser/content_suggestions/BUILD.gn b/ios/chrome/browser/content_suggestions/BUILD.gn index ed24baa..a215b22f 100644 --- a/ios/chrome/browser/content_suggestions/BUILD.gn +++ b/ios/chrome/browser/content_suggestions/BUILD.gn
@@ -9,6 +9,7 @@ "content_suggestions_category_wrapper.mm", "content_suggestions_coordinator.h", "content_suggestions_coordinator.mm", + "content_suggestions_header_provider.h", "content_suggestions_mediator.h", "content_suggestions_mediator.mm", "content_suggestions_service_bridge_observer.h",
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_header_provider.h b/ios/chrome/browser/content_suggestions/content_suggestions_header_provider.h new file mode 100644 index 0000000..aef3ff89 --- /dev/null +++ b/ios/chrome/browser/content_suggestions/content_suggestions_header_provider.h
@@ -0,0 +1,15 @@ +// 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_HEADER_PROVIDER_H_ +#define IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_HEADER_PROVIDER_H_ + +// Object providing a header view for the content suggestions. +@protocol ContentSuggestionsHeaderProvider + +- (nullable UIView*)header; + +@end + +#endif // IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_HEADER_PROVIDER_H_
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_mediator.h b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.h index 9b12b04..cdf84a14 100644 --- a/ios/chrome/browser/content_suggestions/content_suggestions_mediator.h +++ b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.h
@@ -25,6 +25,7 @@ } @protocol ContentSuggestionsCommands; +@protocol ContentSuggestionsHeaderProvider; @class ContentSuggestionIdentifier; class GURL; @@ -48,6 +49,9 @@ @property(nonatomic, weak, nullable) id<ContentSuggestionsCommands> commandHandler; +@property(nonatomic, weak, nullable) id<ContentSuggestionsHeaderProvider> + headerProvider; + // Blacklists the URL from the Most Visited sites. - (void)blacklistMostVisitedURL:(GURL)URL;
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm index b179946..e0d4028 100644 --- a/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm +++ b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
@@ -15,6 +15,7 @@ #include "components/ntp_tiles/most_visited_sites.h" #include "components/ntp_tiles/ntp_tile.h" #import "ios/chrome/browser/content_suggestions/content_suggestions_category_wrapper.h" +#import "ios/chrome/browser/content_suggestions/content_suggestions_header_provider.h" #import "ios/chrome/browser/content_suggestions/content_suggestions_service_bridge_observer.h" #import "ios/chrome/browser/content_suggestions/mediator_util.h" #include "ios/chrome/browser/ntp_tiles/most_visited_sites_observer_bridge.h" @@ -60,6 +61,9 @@ // the callback). Those items are up to date with the model. @property(nonatomic, strong) NSMutableArray<ContentSuggestionsMostVisitedItem*>* freshMostVisitedItems; +// Section Info for the logo and omnibox section. +@property(nonatomic, strong) + ContentSuggestionsSectionInformation* logoSectionInfo; // Section Info for the Most Visited section. @property(nonatomic, strong) ContentSuggestionsSectionInformation* mostVisitedSectionInfo; @@ -83,6 +87,7 @@ @synthesize mostVisitedItems = _mostVisitedItems; @synthesize freshMostVisitedItems = _freshMostVisitedItems; +@synthesize logoSectionInfo = _logoSectionInfo; @synthesize mostVisitedSectionInfo = _mostVisitedSectionInfo; @synthesize recordedPageImpression = _recordedPageImpression; @synthesize contentService = _contentService; @@ -90,6 +95,7 @@ @synthesize sectionInformationByCategory = _sectionInformationByCategory; @synthesize attributesProvider = _attributesProvider; @synthesize commandHandler = _commandHandler; +@synthesize headerProvider = _headerProvider; #pragma mark - Public @@ -110,6 +116,7 @@ largeIconService:largeIconService]; _mostVisitedSectionInfo = MostVisitedSectionInformation(); + _logoSectionInfo = LogoSectionInformation(); _mostVisitedSites = std::move(mostVisitedSites); _mostVisitedBridge = base::MakeUnique<ntp_tiles::MostVisitedSitesObserverBridge>(self); @@ -135,6 +142,8 @@ NSMutableArray<ContentSuggestionsSectionInformation*>* sectionsInfo = [NSMutableArray array]; + [sectionsInfo addObject:self.logoSectionInfo]; + if (self.mostVisitedItems.count > 0) { [sectionsInfo addObject:self.mostVisitedSectionInfo]; } @@ -159,7 +168,9 @@ NSMutableArray<CSCollectionViewItem*>* convertedSuggestions = [NSMutableArray array]; - if (sectionInfo == self.mostVisitedSectionInfo) { + if (sectionInfo == self.logoSectionInfo) { + // TODO(crbug.com/732416): Add promo. + } else if (sectionInfo == self.mostVisitedSectionInfo) { [convertedSuggestions addObjectsFromArray:self.mostVisitedItems]; } else { ntp_snippets::Category category = @@ -245,14 +256,14 @@ ContentSuggestionsSectionInformation* sectionInfo = item.suggestionIdentifier.sectionInfo; GURL url; - if (![self isRelatedToContentSuggestionsService:sectionInfo]) { - ContentSuggestionsMostVisitedItem* mostVisited = - base::mac::ObjCCast<ContentSuggestionsMostVisitedItem>(item); - url = mostVisited.URL; - } else { + if ([self isRelatedToContentSuggestionsService:sectionInfo]) { ContentSuggestionsItem* suggestionItem = base::mac::ObjCCast<ContentSuggestionsItem>(item); url = suggestionItem.URL; + } else if (sectionInfo == self.mostVisitedSectionInfo) { + ContentSuggestionsMostVisitedItem* mostVisited = + base::mac::ObjCCast<ContentSuggestionsMostVisitedItem>(item); + url = mostVisited.URL; } [self.attributesProvider fetchFaviconAttributesForURL:url completion:completion]; @@ -298,6 +309,10 @@ self.contentService->DismissSuggestion(suggestion_id); } +- (UIView*)headerView { + return [self.headerProvider header]; +} + #pragma mark - ContentSuggestionsServiceObserver - (void)contentSuggestionsService: @@ -473,7 +488,8 @@ // content suggestions service. - (BOOL)isRelatedToContentSuggestionsService: (ContentSuggestionsSectionInformation*)sectionInfo { - return sectionInfo != self.mostVisitedSectionInfo; + return sectionInfo != self.mostVisitedSectionInfo && + sectionInfo != self.logoSectionInfo; } // Replaces the Most Visited items currently displayed by the most recent ones.
diff --git a/ios/chrome/browser/content_suggestions/mediator_util.h b/ios/chrome/browser/content_suggestions/mediator_util.h index aef6432..74cc22e2 100644 --- a/ios/chrome/browser/content_suggestions/mediator_util.h +++ b/ios/chrome/browser/content_suggestions/mediator_util.h
@@ -54,6 +54,10 @@ ContentSuggestionsCategoryWrapper* category, const std::string& id_in_category); +// Creates and returns a SectionInfo for the section containing the logo and +// omnibox. +ContentSuggestionsSectionInformation* LogoSectionInformation(); + // Creates and returns a SectionInfo for the Most Visited section. ContentSuggestionsSectionInformation* MostVisitedSectionInformation();
diff --git a/ios/chrome/browser/content_suggestions/mediator_util.mm b/ios/chrome/browser/content_suggestions/mediator_util.mm index bd8a1d2..3e02880e 100644 --- a/ios/chrome/browser/content_suggestions/mediator_util.mm +++ b/ios/chrome/browser/content_suggestions/mediator_util.mm
@@ -100,6 +100,18 @@ return ntp_snippets::ContentSuggestion::ID(category.category, id_in_category); } +ContentSuggestionsSectionInformation* LogoSectionInformation() { + ContentSuggestionsSectionInformation* sectionInfo = + [[ContentSuggestionsSectionInformation alloc] + initWithSectionID:ContentSuggestionsSectionLogo]; + sectionInfo.title = nil; + sectionInfo.footerTitle = nil; + sectionInfo.showIfEmpty = YES; + sectionInfo.layout = ContentSuggestionsSectionLayoutCustom; + + return sectionInfo; +} + ContentSuggestionsSectionInformation* MostVisitedSectionInformation() { ContentSuggestionsSectionInformation* sectionInfo = [[ContentSuggestionsSectionInformation alloc]
diff --git a/ios/chrome/browser/passwords/BUILD.gn b/ios/chrome/browser/passwords/BUILD.gn index b149553..34da56f 100644 --- a/ios/chrome/browser/passwords/BUILD.gn +++ b/ios/chrome/browser/passwords/BUILD.gn
@@ -13,8 +13,6 @@ "ios_chrome_password_manager_driver.mm", "ios_chrome_password_manager_infobar_delegate.h", "ios_chrome_password_manager_infobar_delegate.mm", - "ios_chrome_password_manager_setting_migrator_service_factory.cc", - "ios_chrome_password_manager_setting_migrator_service_factory.h", "ios_chrome_password_store_factory.cc", "ios_chrome_password_store_factory.h", "ios_chrome_save_password_infobar_delegate.h",
diff --git a/ios/chrome/browser/passwords/ios_chrome_password_manager_setting_migrator_service_factory.cc b/ios/chrome/browser/passwords/ios_chrome_password_manager_setting_migrator_service_factory.cc deleted file mode 100644 index e891a39..0000000 --- a/ios/chrome/browser/passwords/ios_chrome_password_manager_setting_migrator_service_factory.cc +++ /dev/null
@@ -1,53 +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. - -#include "ios/chrome/browser/passwords/ios_chrome_password_manager_setting_migrator_service_factory.h" - -#include "base/memory/ptr_util.h" -#include "base/memory/singleton.h" -#include "components/keyed_service/ios/browser_state_dependency_manager.h" -#include "components/password_manager/sync/browser/password_manager_setting_migrator_service.h" -#include "components/sync/driver/sync_service.h" -#include "ios/chrome/browser/browser_state/browser_state_otr_helper.h" -#include "ios/chrome/browser/browser_state/chrome_browser_state.h" - -// static -password_manager::PasswordManagerSettingMigratorService* -IOSChromePasswordManagerSettingMigratorServiceFactory::GetForBrowserState( - ios::ChromeBrowserState* browser_state) { - return static_cast<password_manager::PasswordManagerSettingMigratorService*>( - GetInstance()->GetServiceForBrowserState(browser_state, true)); -} - -// static -IOSChromePasswordManagerSettingMigratorServiceFactory* -IOSChromePasswordManagerSettingMigratorServiceFactory::GetInstance() { - return base::Singleton< - IOSChromePasswordManagerSettingMigratorServiceFactory>::get(); -} - -IOSChromePasswordManagerSettingMigratorServiceFactory:: - IOSChromePasswordManagerSettingMigratorServiceFactory() - : BrowserStateKeyedServiceFactory( - "PasswordManagerSettingMigratorService", - BrowserStateDependencyManager::GetInstance()) {} - -IOSChromePasswordManagerSettingMigratorServiceFactory:: - ~IOSChromePasswordManagerSettingMigratorServiceFactory() {} - -std::unique_ptr<KeyedService> -IOSChromePasswordManagerSettingMigratorServiceFactory::BuildServiceInstanceFor( - web::BrowserState* context) const { - ios::ChromeBrowserState* browser_state = - ios::ChromeBrowserState::FromBrowserState(context); - return base::MakeUnique< - password_manager::PasswordManagerSettingMigratorService>( - browser_state->GetSyncablePrefs()); -} - -web::BrowserState* -IOSChromePasswordManagerSettingMigratorServiceFactory::GetBrowserStateToUse( - web::BrowserState* context) const { - return GetBrowserStateRedirectedInIncognito(context); -}
diff --git a/ios/chrome/browser/passwords/ios_chrome_password_manager_setting_migrator_service_factory.h b/ios/chrome/browser/passwords/ios_chrome_password_manager_setting_migrator_service_factory.h deleted file mode 100644 index 8b1a9d4..0000000 --- a/ios/chrome/browser/passwords/ios_chrome_password_manager_setting_migrator_service_factory.h +++ /dev/null
@@ -1,50 +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 IOS_CHROME_BROWSER_PASSWORDS_IOS_CHROME_PASSWORD_MANAGER_SETTING_MIGRATOR_SERVICE_FACTORY_H_ -#define IOS_CHROME_BROWSER_PASSWORDS_IOS_CHROME_PASSWORD_MANAGER_SETTING_MIGRATOR_SERVICE_FACTORY_H_ - -#include <memory> - -#include "base/macros.h" -#include "components/keyed_service/ios/browser_state_keyed_service_factory.h" - -enum class ServiceAccessType; - -namespace base { -template <typename T> -struct DefaultSingletonTraits; -} - -namespace ios { -class ChromeBrowserState; -} - -namespace password_manager { -class PasswordManagerSettingMigratorService; -} - -class IOSChromePasswordManagerSettingMigratorServiceFactory - : public BrowserStateKeyedServiceFactory { - public: - static password_manager::PasswordManagerSettingMigratorService* - GetForBrowserState(ios::ChromeBrowserState* browser_state); - - static IOSChromePasswordManagerSettingMigratorServiceFactory* GetInstance(); - - private: - friend struct base::DefaultSingletonTraits< - IOSChromePasswordManagerSettingMigratorServiceFactory>; - - IOSChromePasswordManagerSettingMigratorServiceFactory(); - ~IOSChromePasswordManagerSettingMigratorServiceFactory() override; - - // BrowserStateKeyedServiceFactory: - std::unique_ptr<KeyedService> BuildServiceInstanceFor( - web::BrowserState* context) const override; - web::BrowserState* GetBrowserStateToUse( - web::BrowserState* context) const override; -}; - -#endif // IOS_CHROME_BROWSER_PASSWORDS_IOS_CHROME_PASSWORD_MANAGER_SETTING_MIGRATOR_SERVICE_FACTORY_H_
diff --git a/ios/chrome/browser/prefs/ios_chrome_pref_service_factory.cc b/ios/chrome/browser/prefs/ios_chrome_pref_service_factory.cc index a101567..6d2cf5b 100644 --- a/ios/chrome/browser/prefs/ios_chrome_pref_service_factory.cc +++ b/ios/chrome/browser/prefs/ios_chrome_pref_service_factory.cc
@@ -13,6 +13,7 @@ #include "components/prefs/persistent_pref_store.h" #include "components/prefs/pref_filter.h" #include "components/prefs/pref_service.h" +#include "components/prefs/pref_value_store.h" #include "components/proxy_config/proxy_config_pref_names.h" #include "components/sync_preferences/pref_service_syncable.h" #include "components/sync_preferences/pref_service_syncable_factory.h" @@ -80,5 +81,6 @@ overlay_pref_names.push_back(proxy_config::prefs::kProxy); return base::WrapUnique(pref_service->CreateIncognitoPrefService( nullptr, // incognito_extension_pref_store - overlay_pref_names)); + overlay_pref_names, std::set<PrefValueStore::PrefStoreType>(), nullptr, + nullptr)); }
diff --git a/ios/chrome/browser/ui/content_suggestions/cells/BUILD.gn b/ios/chrome/browser/ui/content_suggestions/cells/BUILD.gn index 4191ed57..4770649 100644 --- a/ios/chrome/browser/ui/content_suggestions/cells/BUILD.gn +++ b/ios/chrome/browser/ui/content_suggestions/cells/BUILD.gn
@@ -29,6 +29,8 @@ "content_suggestions_cell.mm", "content_suggestions_footer_item.h", "content_suggestions_footer_item.mm", + "content_suggestions_header_item.h", + "content_suggestions_header_item.mm", "content_suggestions_most_visited_cell.h", "content_suggestions_most_visited_cell.mm", "content_suggestions_text_item.h", @@ -52,6 +54,7 @@ testonly = true sources = [ "content_suggestions_footer_item_unittest.mm", + "content_suggestions_header_item_unittest.mm", "content_suggestions_item_unittest.mm", "content_suggestions_most_visited_item_unittest.mm", ]
diff --git a/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_item.h b/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_item.h new file mode 100644 index 0000000..49626637 --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_item.h
@@ -0,0 +1,27 @@ +// 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_CELLS_CONTENT_SUGGESTIONS_HEADER_ITEM_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CELLS_CONTENT_SUGGESTIONS_HEADER_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 to display a view cell. +@interface ContentSuggestionsHeaderItem : CollectionViewItem + +// The view to be displayed. +@property(nonatomic, strong) UIView* view; + +@end + +// Corresponding cell. +@interface ContentSuggestionsHeaderCell : MDCCollectionViewCell + +// The header view to be displayed. +@property(nonatomic, strong) UIView* headerView; + +@end + +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CELLS_CONTENT_SUGGESTIONS_HEADER_ITEM_H_
diff --git a/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_item.mm b/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_item.mm new file mode 100644 index 0000000..f15ce98 --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_item.mm
@@ -0,0 +1,53 @@ +// 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/cells/content_suggestions_header_item.h" + +#import "ios/chrome/browser/ui/uikit_ui_util.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +@implementation ContentSuggestionsHeaderItem + +@synthesize view; + +- (instancetype)initWithType:(NSInteger)type { + self = [super initWithType:type]; + if (self) { + self.cellClass = [ContentSuggestionsHeaderCell class]; + } + return self; +} + +- (void)configureCell:(ContentSuggestionsHeaderCell*)cell { + [super configureCell:cell]; + [cell setHeaderView:self.view]; +} + +@end + +@implementation ContentSuggestionsHeaderCell + +@synthesize headerView = _headerView; + +- (void)setHeaderView:(UIView*)header { + [_headerView removeFromSuperview]; + _headerView = header; + + if (!header) + return; + + header.translatesAutoresizingMaskIntoConstraints = NO; + [self.contentView addSubview:header]; + AddSameConstraints(self.contentView, header); +} + +- (void)prepareForReuse { + [super prepareForReuse]; + self.headerView = nil; +} + +@end
diff --git a/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_item_unittest.mm b/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_item_unittest.mm new file mode 100644 index 0000000..166f301 --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_item_unittest.mm
@@ -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. + +#import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_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 { + +TEST(ContentSuggestionsHeaderItemTest, CellClass) { + // Setup. + ContentSuggestionsHeaderItem* item = + [[ContentSuggestionsHeaderItem alloc] initWithType:0]; + + // Action. + ContentSuggestionsHeaderCell* cell = [[[item cellClass] alloc] init]; + + // Test. + EXPECT_EQ([ContentSuggestionsHeaderCell class], [cell class]); +} + +TEST(ContentSuggestionsHeaderItemTest, Configure) { + // Setup. + UIView* view = [[UIView alloc] init]; + ContentSuggestionsHeaderItem* item = + [[ContentSuggestionsHeaderItem alloc] initWithType:0]; + item.view = view; + ContentSuggestionsHeaderCell* cell = [[[item cellClass] alloc] init]; + + // Action. + [item configureCell:cell]; + + // Test. + ASSERT_EQ(1U, [cell.contentView.subviews count]); + EXPECT_EQ(view, cell.contentView.subviews[0]); +} +}
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h index 9d49ce8..6473cd9 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h
@@ -65,6 +65,7 @@ // Adds an empty item to this |section| and returns its index path. The updater // does not do any check about the number of elements in the section. +// Returns nil if there is no empty item for this section. - (NSIndexPath*)addEmptyItemForSection:(NSInteger)section; // Returns whether |section| contains the Most Visited tiles.
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm index 8545fa5..82fd4ac 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm
@@ -14,6 +14,7 @@ #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/content_suggestions/cells/content_suggestions_footer_item.h" +#import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_header_item.h" #import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_text_item.h" #import "ios/chrome/browser/ui/content_suggestions/cells/suggested_content.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h" @@ -43,6 +44,7 @@ ItemTypeEmpty, ItemTypeReadingList, ItemTypeMostVisited, + ItemTypePromo, ItemTypeUnknown, }; @@ -52,6 +54,7 @@ SectionIdentifierArticles = kSectionIdentifierEnumZero, SectionIdentifierReadingList, SectionIdentifierMostVisited, + SectionIdentifierLogo, SectionIdentifierDefault, }; @@ -71,7 +74,7 @@ return ContentSuggestionTypeEmpty; } -// Returns the section identifier corresponding to the section |info|. +// Returns the item type corresponding to the section |info|. ItemType ItemTypeForInfo(ContentSuggestionsSectionInformation* info) { switch (info.sectionID) { case ContentSuggestionsSectionArticles: @@ -80,6 +83,8 @@ return ItemTypeReadingList; case ContentSuggestionsSectionMostVisited: return ItemTypeMostVisited; + case ContentSuggestionsSectionLogo: + return ItemTypePromo; case ContentSuggestionsSectionUnknown: return ItemTypeUnknown; @@ -96,6 +101,8 @@ return SectionIdentifierReadingList; case ContentSuggestionsSectionMostVisited: return SectionIdentifierMostVisited; + case ContentSuggestionsSectionLogo: + return SectionIdentifierLogo; case ContentSuggestionsSectionUnknown: return SectionIdentifierDefault; @@ -301,11 +308,12 @@ if ([model hasSectionForSectionIdentifier:sectionIdentifier] && [model numberOfItemsInSection:[model sectionForSectionIdentifier: sectionIdentifier]] == 0) { - [indexPaths - addObject:[self - addEmptyItemForSection:[model - sectionForSectionIdentifier: - sectionIdentifier]]]; + NSIndexPath* emptyItemIndexPath = + [self addEmptyItemForSection: + [model sectionForSectionIdentifier:sectionIdentifier]]; + if (emptyItemIndexPath) { + [indexPaths addObject:emptyItemIndexPath]; + } } return indexPaths; } @@ -353,6 +361,9 @@ orderedSectionsInfo) { NSInteger orderedSectionIdentifier = SectionIdentifierForInfo(orderedSectionInfo); + if (orderedSectionIdentifier == sectionIdentifier) { + continue; + } if ([model hasSectionForSectionIdentifier:orderedSectionIdentifier]) { sectionIndex++; } @@ -362,7 +373,11 @@ self.sectionInfoBySectionIdentifier[@(sectionIdentifier)] = sectionInfo; [addedSectionIdentifiers addIndex:sectionIdentifier]; - [self addHeaderIfNeeded:sectionInfo]; + if (sectionIdentifier == SectionIdentifierLogo) { + [self addLogoHeaderIfNeeded]; + } else { + [self addHeaderIfNeeded:sectionInfo]; + } [self addFooterIfNeeded:sectionInfo]; } @@ -383,6 +398,9 @@ self.sectionInfoBySectionIdentifier[@(sectionIdentifier)]; CSCollectionViewItem* item = [self emptyItemForSectionInfo:sectionInfo]; + if (!item) { + return nil; + } return [self addItem:item toSectionWithIdentifier:sectionIdentifier]; } @@ -501,6 +519,20 @@ } } +// Adds the header for the first section, containing the logo and the omnibox, +// if there is no header for the section. +- (void)addLogoHeaderIfNeeded { + if (![self.collectionViewController.collectionViewModel + headerForSectionWithIdentifier:SectionIdentifierLogo]) { + ContentSuggestionsHeaderItem* header = + [[ContentSuggestionsHeaderItem alloc] initWithType:ItemTypeHeader]; + header.view = self.dataSource.headerView; + [self.collectionViewController.collectionViewModel + setHeader:header + forSectionWithIdentifier:SectionIdentifierLogo]; + } +} + // Resets the models, removing the current CollectionViewItem and the // SectionInfo. - (void)resetModels { @@ -599,8 +631,11 @@ // Returns a item to be displayed when the section identified by |sectionInfo| // is empty. +// Returns nil if there is no empty item for this section info. - (CSCollectionViewItem*)emptyItemForSectionInfo: (ContentSuggestionsSectionInformation*)sectionInfo { + if (!sectionInfo.emptyText) + return nil; ContentSuggestionsTextItem* item = [[ContentSuggestionsTextItem alloc] initWithType:ItemTypeEmpty]; item.text = l10n_util::GetNSString(IDS_NTP_TITLE_NO_SUGGESTIONS);
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater_unittest.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater_unittest.mm index edfdf4f..94e30a29 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater_unittest.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater_unittest.mm
@@ -4,6 +4,7 @@ #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h" +#import "ios/chrome/browser/ui/collection_view/cells/collection_view_text_item.h" #import "ios/chrome/browser/ui/collection_view/collection_view_model.h" #import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_text_item.h" #import "ios/chrome/browser/ui/content_suggestions/cells/suggested_content.h" @@ -22,6 +23,7 @@ TEST(ContentSuggestionsCollectionUpdaterTest, addEmptyItemToEmptySection) { // Setup. + NSString* emptyString = @"test empty"; ContentSuggestionsCollectionUpdater* updater = [[ContentSuggestionsCollectionUpdater alloc] initWithDataSource:nil]; CollectionViewModel* model = [[CollectionViewModel alloc] init]; @@ -36,6 +38,7 @@ [[ContentSuggestionsSectionInformation alloc] initWithSectionID:ContentSuggestionsSectionArticles]; suggestion.suggestionIdentifier.sectionInfo.showIfEmpty = YES; + suggestion.suggestionIdentifier.sectionInfo.emptyText = emptyString; [updater addSectionsForSectionInfoToModel:@[ suggestion.suggestionIdentifier.sectionInfo ]]; @@ -46,7 +49,42 @@ // Test. EXPECT_TRUE([[NSIndexPath indexPathForItem:0 inSection:0] isEqual:addedItem]); + NSArray* items = [model + itemsInSectionWithIdentifier:[model sectionIdentifierForSection:0]]; ASSERT_EQ(1, [model numberOfItemsInSection:0]); + ContentSuggestionsTextItem* item = items[0]; + EXPECT_EQ(emptyString, item.detailText); +} + +TEST(ContentSuggestionsCollectionUpdaterTest, + addEmptyItemToSectionWithoutText) { + // Setup. + ContentSuggestionsCollectionUpdater* updater = + [[ContentSuggestionsCollectionUpdater alloc] initWithDataSource:nil]; + CollectionViewModel* model = [[CollectionViewModel alloc] init]; + id mockCollection = OCMClassMock([ContentSuggestionsViewController class]); + OCMStub([mockCollection collectionViewModel]).andReturn(model); + updater.collectionViewController = mockCollection; + + CollectionViewItem<SuggestedContent>* suggestion = + [[ContentSuggestionsTextItem alloc] initWithType:kItemTypeEnumZero]; + suggestion.suggestionIdentifier = [[ContentSuggestionIdentifier alloc] init]; + suggestion.suggestionIdentifier.sectionInfo = + [[ContentSuggestionsSectionInformation alloc] + initWithSectionID:ContentSuggestionsSectionArticles]; + suggestion.suggestionIdentifier.sectionInfo.showIfEmpty = YES; + suggestion.suggestionIdentifier.sectionInfo.emptyText = nil; + [updater addSectionsForSectionInfoToModel:@[ + suggestion.suggestionIdentifier.sectionInfo + ]]; + ASSERT_EQ(0, [model numberOfItemsInSection:0]); + + // Action. + NSIndexPath* addedItem = [updater addEmptyItemForSection:0]; + + // Test. + EXPECT_EQ(nil, addedItem); + ASSERT_EQ(0, [model numberOfItemsInSection:0]); } TEST(ContentSuggestionsCollectionUpdaterTest, addEmptyItemToSection) { @@ -65,6 +103,7 @@ [[ContentSuggestionsSectionInformation alloc] initWithSectionID:ContentSuggestionsSectionArticles]; suggestion.suggestionIdentifier.sectionInfo.showIfEmpty = YES; + suggestion.suggestionIdentifier.sectionInfo.emptyText = @"empty"; [updater addSectionsForSectionInfoToModel:@[ suggestion.suggestionIdentifier.sectionInfo ]];
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h index 7552be8c..1c3fe93 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h
@@ -67,6 +67,9 @@ - (void)dismissSuggestion: (nonnull ContentSuggestionIdentifier*)suggestionIdentifier; +// Returns the header view containing the logo and omnibox to be displayed. +- (nullable UIView*)headerView; + @end #endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_DATA_SOURCE_H_
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm index f0312887..a45fee6 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm
@@ -272,6 +272,19 @@ return [UIColor whiteColor]; } +- (CGSize)collectionView:(UICollectionView*)collectionView + layout: + (UICollectionViewLayout*)collectionViewLayout + referenceSizeForHeaderInSection:(NSInteger)section { + // TODO(crbug.com/635604): Once the headers support dynamic sizing, use it + // instead of this. + if (section == 0) + return CGSizeMake(0, 270); + return [super collectionView:collectionView + layout:collectionViewLayout + referenceSizeForHeaderInSection:section]; +} + - (BOOL)collectionView:(nonnull UICollectionView*)collectionView shouldHideItemBackgroundAtIndexPath:(nonnull NSIndexPath*)indexPath { return @@ -371,7 +384,8 @@ NSIndexPath* emptyItem = [self.collectionUpdater addEmptyItemForSection:section]; - [self.collectionView insertItemsAtIndexPaths:@[ emptyItem ]]; + if (emptyItem) + [self.collectionView insertItemsAtIndexPaths:@[ emptyItem ]]; } // Tells WebToolbarController to resign focus to the omnibox.
diff --git a/ios/chrome/browser/ui/content_suggestions/identifier/content_suggestions_section_information.h b/ios/chrome/browser/ui/content_suggestions/identifier/content_suggestions_section_information.h index e2aea5a..841fd56 100644 --- a/ios/chrome/browser/ui/content_suggestions/identifier/content_suggestions_section_information.h +++ b/ios/chrome/browser/ui/content_suggestions/identifier/content_suggestions_section_information.h
@@ -18,7 +18,8 @@ // This enum is used for identifying the section. All section should have a // different ID. typedef NS_ENUM(NSInteger, ContentSuggestionsSectionID) { - ContentSuggestionsSectionMostVisited = 0, + ContentSuggestionsSectionLogo = 0, + ContentSuggestionsSectionMostVisited, ContentSuggestionsSectionArticles, ContentSuggestionsSectionReadingList,
diff --git a/ios/showcase/content_suggestions/sc_content_suggestions_data_source.mm b/ios/showcase/content_suggestions/sc_content_suggestions_data_source.mm index 78a1ac5..b8b4740c 100644 --- a/ios/showcase/content_suggestions/sc_content_suggestions_data_source.mm +++ b/ios/showcase/content_suggestions/sc_content_suggestions_data_source.mm
@@ -28,6 +28,9 @@ @interface SCContentSuggestionsDataSource ()<ContentSuggestionsImageFetcher> +// Section Info of type Logo header. Created lazily. +@property(nonatomic, strong) + ContentSuggestionsSectionInformation* logoHeaderSection; // Section Info of type MostVisited. Created lazily. @property(nonatomic, strong) ContentSuggestionsSectionInformation* mostVisitedSection; @@ -43,6 +46,7 @@ @implementation SCContentSuggestionsDataSource @synthesize dataSink = _dataSink; +@synthesize logoHeaderSection = _logoHeaderSection; @synthesize mostVisitedSection = _mostVisitedSection; @synthesize readingListSection = _readingListSection; @synthesize articleSection = _articleSection; @@ -61,7 +65,8 @@ - (NSArray<ContentSuggestionsSectionInformation*>*)sectionsInfo { return @[ - self.mostVisitedSection, self.readingListSection, self.articleSection + self.logoHeaderSection, self.mostVisitedSection, self.readingListSection, + self.articleSection ]; } @@ -103,6 +108,9 @@ } return items; } + case ContentSuggestionsSectionLogo: { + return @[]; + } case ContentSuggestionsSectionUnknown: return @[]; } @@ -130,6 +138,10 @@ - (void)dismissSuggestion:(ContentSuggestionIdentifier*)suggestionIdentifier { } +- (UIView*)headerView { + return nil; +} + #pragma mark - ContentSuggestionsImageFetcher - (void)fetchImageForSuggestion: @@ -142,11 +154,27 @@ #pragma mark - Property +- (ContentSuggestionsSectionInformation*)logoHeaderSection { + if (!_logoHeaderSection) { + _logoHeaderSection = [[ContentSuggestionsSectionInformation alloc] + initWithSectionID:ContentSuggestionsSectionLogo]; + _logoHeaderSection.showIfEmpty = YES; + _logoHeaderSection.layout = ContentSuggestionsSectionLayoutCustom; + _logoHeaderSection.title = nil; + _logoHeaderSection.footerTitle = nil; + _logoHeaderSection.emptyText = nil; + } + return _logoHeaderSection; +} + - (ContentSuggestionsSectionInformation*)mostVisitedSection { if (!_mostVisitedSection) { _mostVisitedSection = [[ContentSuggestionsSectionInformation alloc] initWithSectionID:ContentSuggestionsSectionMostVisited]; _mostVisitedSection.layout = ContentSuggestionsSectionLayoutCustom; + _mostVisitedSection.title = nil; + _mostVisitedSection.footerTitle = nil; + _mostVisitedSection.emptyText = nil; } return _mostVisitedSection; }
diff --git a/ios/showcase/content_suggestions/sc_content_suggestions_egtest.mm b/ios/showcase/content_suggestions/sc_content_suggestions_egtest.mm index 4060db73..08a30c1 100644 --- a/ios/showcase/content_suggestions/sc_content_suggestions_egtest.mm +++ b/ios/showcase/content_suggestions/sc_content_suggestions_egtest.mm
@@ -35,15 +35,20 @@ matcher, nil); } -// Select the cell with the |ID| by scrolling the collection. -GREYElementInteraction* CellWithID(NSString* ID) { +// Select the cell with the |matcher| by scrolling the collection. +GREYElementInteraction* CellWithMatcher(id<GREYMatcher> matcher) { return [[EarlGrey - selectElementWithMatcher:grey_allOf(grey_accessibilityID(ID), - grey_sufficientlyVisible(), nil)] + selectElementWithMatcher:grey_allOf(matcher, grey_sufficientlyVisible(), + nil)] usingSearchAction:grey_scrollInDirection(kGREYDirectionDown, 150) onElementWithMatcher:grey_kindOfClass([UICollectionView class])]; } +// Select the cell with the |ID| by scrolling the collection. +GREYElementInteraction* CellWithID(NSString* ID) { + return CellWithMatcher(grey_accessibilityID(ID)); +} + // Returns the string displayed when the Reading List section is empty. NSString* ReadingListEmptySection() { return [NSString @@ -63,14 +68,12 @@ // Tests launching ContentSuggestionsViewController. - (void)testLaunch { showcase_utils::Open(@"ContentSuggestionsViewController"); - [[EarlGrey - selectElementWithMatcher:chrome_test_util::ButtonWithAccessibilityLabelId( - IDS_IOS_CONTENT_SUGGESTIONS_FOOTER_TITLE)] + [CellWithMatcher(chrome_test_util::ButtonWithAccessibilityLabelId( + IDS_IOS_CONTENT_SUGGESTIONS_FOOTER_TITLE)) assertWithMatcher:grey_interactable()]; - [[EarlGrey selectElementWithMatcher: - chrome_test_util::StaticTextWithAccessibilityLabelId( - IDS_NTP_ARTICLE_SUGGESTIONS_SECTION_HEADER)] - assertWithMatcher:grey_sufficientlyVisible()]; + [CellWithMatcher(chrome_test_util::StaticTextWithAccessibilityLabelId( + IDS_NTP_ARTICLE_SUGGESTIONS_SECTION_HEADER)) + assertWithMatcher:grey_notNil()]; showcase_utils::Close(); }
diff --git a/mojo/public/cpp/bindings/sync_call_restrictions.h b/mojo/public/cpp/bindings/sync_call_restrictions.h index 6ed1946..7eef38c5 100644 --- a/mojo/public/cpp/bindings/sync_call_restrictions.h +++ b/mojo/public/cpp/bindings/sync_call_restrictions.h
@@ -15,6 +15,10 @@ #define ENABLE_SYNC_CALL_RESTRICTIONS 0 #endif +namespace sync_preferences { +class PrefServiceSyncable; +} + namespace content { class BrowserTestBase; } @@ -73,6 +77,8 @@ friend class leveldb::LevelDBMojoProxy; // Pref service connection is sync at startup. friend class prefs::PersistentPrefStoreClient; + // Incognito pref service instances are created synchronously. + friend class sync_preferences::PrefServiceSyncable; // END ALLOWED USAGE.
diff --git a/services/preferences/public/cpp/BUILD.gn b/services/preferences/public/cpp/BUILD.gn index 53ead75..824f4599 100644 --- a/services/preferences/public/cpp/BUILD.gn +++ b/services/preferences/public/cpp/BUILD.gn
@@ -12,14 +12,14 @@ "pref_registry_serializer.h", "pref_service_factory.cc", "pref_service_factory.h", - "pref_store_adapter.cc", - "pref_store_adapter.h", "pref_store_client.cc", "pref_store_client.h", "pref_store_client_mixin.cc", "pref_store_client_mixin.h", "pref_store_impl.cc", "pref_store_impl.h", + "registering_delegate.cc", + "registering_delegate.h", "scoped_pref_update.cc", "scoped_pref_update.h", ]
diff --git a/services/preferences/public/cpp/pref_store_adapter.cc b/services/preferences/public/cpp/pref_store_adapter.cc deleted file mode 100644 index 2cfe85a..0000000 --- a/services/preferences/public/cpp/pref_store_adapter.cc +++ /dev/null
@@ -1,40 +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. - -#include "services/preferences/public/cpp/pref_store_adapter.h" - -namespace prefs { - -PrefStoreAdapter::PrefStoreAdapter(scoped_refptr<PrefStore> pref_store, - std::unique_ptr<PrefStoreImpl> impl) - : pref_store_(std::move(pref_store)), impl_(std::move(impl)) {} - -void PrefStoreAdapter::AddObserver(PrefStore::Observer* observer) { - pref_store_->AddObserver(observer); -} - -void PrefStoreAdapter::RemoveObserver(PrefStore::Observer* observer) { - pref_store_->RemoveObserver(observer); -} - -bool PrefStoreAdapter::HasObservers() const { - return pref_store_->HasObservers(); -} - -bool PrefStoreAdapter::IsInitializationComplete() const { - return pref_store_->IsInitializationComplete(); -} - -bool PrefStoreAdapter::GetValue(const std::string& key, - const base::Value** result) const { - return pref_store_->GetValue(key, result); -} - -std::unique_ptr<base::DictionaryValue> PrefStoreAdapter::GetValues() const { - return pref_store_->GetValues(); -} - -PrefStoreAdapter::~PrefStoreAdapter() = default; - -} // namespace prefs
diff --git a/services/preferences/public/cpp/pref_store_adapter.h b/services/preferences/public/cpp/pref_store_adapter.h deleted file mode 100644 index c734606..0000000 --- a/services/preferences/public/cpp/pref_store_adapter.h +++ /dev/null
@@ -1,44 +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 SERVICES_PREFERENCES_PUBLIC_CPP_PREF_STORE_ADAPTER_H_ -#define SERVICES_PREFERENCES_PUBLIC_CPP_PREF_STORE_ADAPTER_H_ - -#include <memory> - -#include "base/macros.h" -#include "base/values.h" -#include "components/prefs/pref_store.h" -#include "services/preferences/public/cpp/pref_store_impl.h" - -namespace prefs { - -// Ties the lifetime of a |PrefStoreImpl| to a |PrefStore|. Otherwise acts as a -// |PrefStore|, forwarding all calls to the wrapped |PrefStore|. -class PrefStoreAdapter : public PrefStore { - public: - PrefStoreAdapter(scoped_refptr<PrefStore> pref_store, - std::unique_ptr<PrefStoreImpl> impl); - - // PrefStore: - void AddObserver(PrefStore::Observer* observer) override; - void RemoveObserver(PrefStore::Observer* observer) override; - bool HasObservers() const override; - bool IsInitializationComplete() const override; - bool GetValue(const std::string& key, - const base::Value** result) const override; - std::unique_ptr<base::DictionaryValue> GetValues() const override; - - private: - ~PrefStoreAdapter() override; - - scoped_refptr<PrefStore> pref_store_; - std::unique_ptr<PrefStoreImpl> impl_; - - DISALLOW_COPY_AND_ASSIGN(PrefStoreAdapter); -}; - -} // namespace prefs - -#endif // SERVICES_PREFERENCES_PUBLIC_CPP_PREF_STORE_ADAPTER_H_
diff --git a/services/preferences/public/cpp/registering_delegate.cc b/services/preferences/public/cpp/registering_delegate.cc new file mode 100644 index 0000000..e90b494 --- /dev/null +++ b/services/preferences/public/cpp/registering_delegate.cc
@@ -0,0 +1,50 @@ +// Copyright (c) 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 "services/preferences/public/cpp/registering_delegate.h" + +#include "services/preferences/public/cpp/pref_store_impl.h" + +namespace sync_preferences { + +RegisteringDelegate::RegisteringDelegate( + prefs::mojom::PrefStoreRegistryPtr registry) + : registry_(std::move(registry)) {} + +RegisteringDelegate::~RegisteringDelegate() = default; + +void RegisteringDelegate::Init(PrefStore* managed_prefs, + PrefStore* supervised_user_prefs, + PrefStore* extension_prefs, + PrefStore* command_line_prefs, + PrefStore* user_prefs, + PrefStore* recommended_prefs, + PrefStore* default_prefs, + PrefNotifier* pref_notifier) { + RegisterPrefStore(managed_prefs, PrefValueStore::MANAGED_STORE); + RegisterPrefStore(supervised_user_prefs, + PrefValueStore::SUPERVISED_USER_STORE); + RegisterPrefStore(extension_prefs, PrefValueStore::EXTENSION_STORE); + RegisterPrefStore(command_line_prefs, PrefValueStore::COMMAND_LINE_STORE); + RegisterPrefStore(recommended_prefs, PrefValueStore::RECOMMENDED_STORE); +} + +void RegisteringDelegate::UpdateCommandLinePrefStore( + PrefStore* command_line_prefs) { + // TODO(tibell): Once we have a way to deregister stores, do so here. At the + // moment only local state uses this and local state doesn't use the pref + // service yet. +} + +void RegisteringDelegate::RegisterPrefStore( + scoped_refptr<PrefStore> backing_pref_store, + PrefValueStore::PrefStoreType type) { + if (!backing_pref_store) + return; + + impls_.push_back( + prefs::PrefStoreImpl::Create(registry_.get(), backing_pref_store, type)); +} + +} // namespace sync_preferences
diff --git a/services/preferences/public/cpp/registering_delegate.h b/services/preferences/public/cpp/registering_delegate.h new file mode 100644 index 0000000..c77665b8 --- /dev/null +++ b/services/preferences/public/cpp/registering_delegate.h
@@ -0,0 +1,48 @@ +// Copyright (c) 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 <memory> +#include <vector> + +#include "base/memory/ref_counted.h" +#include "components/prefs/pref_value_store.h" +#include "services/preferences/public/interfaces/preferences.mojom.h" + +class PrefStore; +class PrefNotifier; + +namespace prefs { +class PrefStoreImpl; +} + +namespace sync_preferences { + +// Registers all provided |PrefStore|s with the pref service. The pref stores +// remaining registered for the life time of |this|. +class RegisteringDelegate : public PrefValueStore::Delegate { + public: + RegisteringDelegate(prefs::mojom::PrefStoreRegistryPtr registry); + ~RegisteringDelegate() override; + + // PrefValueStore::Delegate: + void Init(PrefStore* managed_prefs, + PrefStore* supervised_user_prefs, + PrefStore* extension_prefs, + PrefStore* command_line_prefs, + PrefStore* user_prefs, + PrefStore* recommended_prefs, + PrefStore* default_prefs, + PrefNotifier* pref_notifier) override; + void UpdateCommandLinePrefStore(PrefStore* command_line_prefs) override; + + private: + // Expose the |backing_pref_store| through the prefs service. + void RegisterPrefStore(scoped_refptr<PrefStore> backing_pref_store, + PrefValueStore::PrefStoreType type); + + prefs::mojom::PrefStoreRegistryPtr registry_; + std::vector<std::unique_ptr<prefs::PrefStoreImpl>> impls_; +}; + +} // namespace sync_preferences
diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json index 16ad991..b963981 100644 --- a/testing/variations/fieldtrial_testing_config.json +++ b/testing/variations/fieldtrial_testing_config.json
@@ -2025,23 +2025,6 @@ ] } ], - "PasswordManagerSettingsMigration": [ - { - "platforms": [ - "android", - "chromeos", - "ios", - "linux", - "mac", - "win" - ], - "experiments": [ - { - "name": "Enable" - } - ] - } - ], "PasswordMetadataFilling": [ { "platforms": [
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index c5316cd..4b8a60b 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -2331,8 +2331,6 @@ crbug.com/675540 virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/claim-with-redirect.https.html [ Skip ] crbug.com/658997 external/wpt/service-workers/service-worker/clients-matchall-client-types.https.html [ Skip ] crbug.com/658997 virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/clients-matchall-client-types.https.html [ Skip ] -crbug.com/658997 external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https.html [ Skip ] -crbug.com/658997 virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https.html [ Skip ] crbug.com/435547 http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html [ Skip ] crbug.com/435547 virtual/mojo-loading/http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html [ Skip ]
diff --git a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https-expected.txt new file mode 100644 index 0000000..d6af7056 --- /dev/null +++ b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https-expected.txt
@@ -0,0 +1,4 @@ +This is a testharness.js-based test. +FAIL Verify canvas tainting of fetched image in a Service Worker assert_equals: expected "finish" but got "failure:Result of url:https://web-platform.test:8444/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=cors&url=https%3A%2F%2Fwww1.web-platform.test%3A8444%2Fservice-workers%2Fservice-worker%2Fresources%2Ffetch-access-control.py%3FPNGIMAGE%26ACAOrigin%3Dhttps%3A%2F%2Fweb-platform.test%3A8444&cache cross_origin: use-credentials must be LOAD_ERROR but NOT_TAINTED" +Harness: the test ran to completion. +
diff --git a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt index a5cfa27..d639d286 100644 --- a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt +++ b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt
@@ -1,4 +1,4 @@ This is a testharness.js-based test. -FAIL Verify canvas tainting of fetched image in a Service Worker assert_equals: expected "finish" but got "failure:Result of url:https://www1.web-platform.test:8444/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=same-origin&url=https%3A%2F%2Fweb-platform.test%3A8444%2Fservice-workers%2Fservice-worker%2Fresources%2Ffetch-access-control.py%3FPNGIMAGE cross_origin: must be TAINTED but NOT_TAINTED" +FAIL Verify canvas tainting of fetched image in a Service Worker assert_equals: expected "finish" but got "failure:Result of url:https://web-platform.test:8444/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=cors&url=https%3A%2F%2Fwww1.web-platform.test%3A8444%2Fservice-workers%2Fservice-worker%2Fresources%2Ffetch-access-control.py%3FPNGIMAGE%26ACAOrigin%3Dhttps%3A%2F%2Fweb-platform.test%3A8444 cross_origin: use-credentials must be LOAD_ERROR but NOT_TAINTED" Harness: the test ran to completion.
diff --git a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/fetch-canvas-tainting-iframe.html b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/fetch-canvas-tainting-iframe.html index 75eca56..e100dab 100644 --- a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/fetch-canvas-tainting-iframe.html +++ b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/fetch-canvas-tainting-iframe.html
@@ -159,7 +159,7 @@ remote_image_url + '&mode=same-origin&url=' + encodeURIComponent(image_url), '', - TAINTED), + NOT_TAINTED), create_test_promise( remote_image_url + '&mode=same-origin&url=' + encodeURIComponent(image_url),
diff --git a/third_party/WebKit/LayoutTests/webshare/share-error.html b/third_party/WebKit/LayoutTests/webshare/share-error.html index ce81761c..7b2950c1 100644 --- a/third_party/WebKit/LayoutTests/webshare/share-error.html +++ b/third_party/WebKit/LayoutTests/webshare/share-error.html
@@ -8,9 +8,8 @@ share_test((t, webshare, mock) => { mock.pushShareResult('the title', 'the message', 'data:the url', webshare.ShareError.CANCELED); - // promise_rejects doesn't test the exception message, so just match against undefined. return callWithKeyDown(() => promise_rejects( - t, new DOMException(undefined, 'AbortError'), + t, 'AbortError', navigator.share({title: 'the title', text: 'the message', url: 'data:the url'}))); }, 'share with user cancellation'); @@ -18,7 +17,7 @@ mock.pushShareResult('the title', 'the message', 'data:the url', webshare.ShareError.INTERNAL_ERROR); return callWithKeyDown(() => promise_rejects( - t, new DOMException(undefined, 'AbortError'), + t, 'AbortError', navigator.share({title: 'the title', text: 'the message', url: 'data:the url'}))); }, 'share with invalid url template');
diff --git a/third_party/WebKit/LayoutTests/webshare/share-types.html b/third_party/WebKit/LayoutTests/webshare/share-types.html index 04dd0f6..3bcc658a 100644 --- a/third_party/WebKit/LayoutTests/webshare/share-types.html +++ b/third_party/WebKit/LayoutTests/webshare/share-types.html
@@ -25,19 +25,19 @@ }, 'share of types other than string (expect implicitly converted to string)'); share_test((t, webshare, mock) => { - // A null title should convert into the string 'null' (because the field is + // null fields should convert into the string 'null' (because the field is // not nullable, it just converts to a string like any other type). - mock.pushShareResult('null', '', '', webshare.ShareError.OK); - return callWithKeyDown(() => navigator.share({title: null, text: undefined})); + mock.pushShareResult('null', '', getAbsoluteUrl('null'), + webshare.ShareError.OK); + return callWithKeyDown(() => navigator.share( + {title: null, text: undefined, url: null})); }, 'share of null/undefined dict values'); promise_test(t => { // URL is invalid in that the URL Parser returns failure (port is too // large). const url = 'http://example.com:65536'; - return promise_rejects( - t, new DOMException(undefined, 'TypeError'), - navigator.share({url: url})); + return promise_rejects(t, new TypeError(), navigator.share({url: url})); }, 'share with an invalid URL'); </script>
diff --git a/third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html b/third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html index 038b658..e3c1390 100644 --- a/third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html +++ b/third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html
@@ -4,9 +4,8 @@ <script> promise_test(t => { - // promise_rejects doesn't test the exception message, so just match against undefined. return promise_rejects( - t, new DOMException(undefined, 'SecurityError'), + t, 'SecurityError', navigator.share({title: 'the title', text: 'the message', url: 'data:the url'})); }, 'share without a user gesture');
diff --git a/third_party/WebKit/Source/build/scripts/make_computed_style_base.py b/third_party/WebKit/Source/build/scripts/make_computed_style_base.py index 7cd3e57c..2aee91fb 100755 --- a/third_party/WebKit/Source/build/scripts/make_computed_style_base.py +++ b/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
@@ -27,13 +27,13 @@ # Aligns like double 'ScaleTransformOperation', 'RotateTransformOperation', 'TranslateTransformOperation', 'double', # Aligns like a pointer (can be 32 or 64 bits) - 'NamedGridLinesMap', 'OrderedNamedGridLines', 'NamedGridAreaMap', 'StyleMotionData', 'TransformOperations', + 'NamedGridLinesMap', 'OrderedNamedGridLines', 'NamedGridAreaMap', 'TransformOperations', 'Vector<CSSPropertyID>', 'Vector<GridTrackSize>', 'GridPosition', 'AtomicString', 'RefPtr', 'DataPersistent', 'Persistent', 'std::unique_ptr', 'Vector<String>', 'Font', 'FillLayer', 'NinePieceImage', # Aligns like float - 'TransformOrigin', 'ScrollPadding', 'ScrollSnapMargin', 'LengthBox', 'LengthSize', 'FloatSize', - 'LengthPoint', 'Length', 'TextSizeAdjust', 'TabSize', 'float', + 'StyleOffsetRotation', 'TransformOrigin', 'ScrollPadding', 'ScrollSnapMargin', 'LengthBox', + 'LengthSize', 'FloatSize', 'LengthPoint', 'Length', 'TextSizeAdjust', 'TabSize', 'float', # Aligns like int 'ScrollSnapType', 'ScrollSnapAlign', 'BorderValue', 'StyleColor', 'Color', 'LayoutUnit', 'LineClampValue', 'OutlineValue', 'unsigned', 'size_t', 'int',
diff --git a/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h b/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h index f03044f..be437b4 100644 --- a/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h +++ b/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h
@@ -404,34 +404,6 @@ } template <> -inline CSSIdentifierValue::CSSIdentifierValue(EBackfaceVisibility e) - : CSSValue(kIdentifierClass) { - switch (e) { - case EBackfaceVisibility::kVisible: - value_id_ = CSSValueVisible; - break; - case EBackfaceVisibility::kHidden: - value_id_ = CSSValueHidden; - break; - } -} - -template <> -inline EBackfaceVisibility CSSIdentifierValue::ConvertTo() const { - switch (value_id_) { - case CSSValueVisible: - return EBackfaceVisibility::kVisible; - case CSSValueHidden: - return EBackfaceVisibility::kHidden; - default: - break; - } - - NOTREACHED(); - return EBackfaceVisibility::kHidden; -} - -template <> inline CSSIdentifierValue::CSSIdentifierValue(EFillAttachment e) : CSSValue(kIdentifierClass) { switch (e) {
diff --git a/third_party/WebKit/Source/core/css/CSSProperties.json5 b/third_party/WebKit/Source/core/css/CSSProperties.json5 index a059d82..39f003b 100644 --- a/third_party/WebKit/Source/core/css/CSSProperties.json5 +++ b/third_party/WebKit/Source/core/css/CSSProperties.json5
@@ -634,11 +634,10 @@ }, { name: "backface-visibility", - field_template: "storage_only", - type_name: "EBackfaceVisibility", + field_template: "keyword", field_group: "rare-non-inherited", - default_value: "EBackfaceVisibility::kVisible", - field_size: 1, + default_value: "visible", + keywords: ["visible", "hidden"], }, { name: "background-attachment", @@ -1741,18 +1740,34 @@ converter: "ConvertPositionOrAuto", interpolable: true, runtime_flag: "CSSOffsetPositionAnchor", + field_template: "external", + type_name: "LengthPoint", + include_paths: ["platform/LengthPoint.h"], + field_group: "rare-non-inherited->transform", + default_value: "LengthPoint(Length(kAuto), Length(kAuto))", }, { name: "offset-distance", api_class: true, converter: "ConvertLength", interpolable: true, + field_template: "external", + type_name: "Length", + include_paths: ["platform/Length.h"], + field_group: "rare-non-inherited->transform", + default_value: "Length(0, kFixed)", }, { name: "offset-path", api_class: true, api_methods: ["parseSingleValue"], converter: "ConvertOffsetPath", + field_template: "storage_only", + type_name: "BasicShape", + field_group: "rare-non-inherited->transform", + wrapper_pointer_name: "RefPtr", + default_value: "nullptr", + include_paths: ["core/style/BasicShapes.h"], }, { name: "offset-position", @@ -1761,12 +1776,22 @@ converter: "ConvertPositionOrAuto", interpolable: true, runtime_flag: "CSSOffsetPositionAnchor", + field_template: "external", + type_name: "LengthPoint", + include_paths: ["platform/LengthPoint.h"], + field_group: "rare-non-inherited->transform", + default_value: "LengthPoint(Length(kAuto), Length(kAuto))", }, { name: "offset-rotate", api_class: "CSSPropertyAPIOffsetRotate", converter: "ConvertOffsetRotate", interpolable: true, + field_template: "external", + type_name: "StyleOffsetRotation", + include_paths: ["core/style/StyleOffsetRotation.h"], + field_group: "rare-non-inherited->transform", + default_value: "StyleOffsetRotation(0, kOffsetRotationAuto)", }, // Whether or not we're transparent. {
diff --git a/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 b/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 index b6a530a..4120b96 100644 --- a/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 +++ b/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5
@@ -893,14 +893,6 @@ include_paths: ["platform/transforms/TransformOperations.h"] }, { - name: "Motion", - field_template: "storage_only", - type_name: "StyleMotionData", - field_group: "rare-non-inherited->transform", - default_value: "StyleMotionData(LengthPoint(Length(kAuto), Length(kAuto)), LengthPoint(Length(kAuto), Length(kAuto)), nullptr, Length(0, kFixed), StyleOffsetRotation(0, kOffsetRotationAuto))", - include_paths: ["core/style/StyleMotionData.h"], - }, - { name: "NamedGridColumnLines", field_template: "external", type_name: "NamedGridLinesMap",
diff --git a/third_party/WebKit/Source/core/dom/AXObjectCache.cpp b/third_party/WebKit/Source/core/dom/AXObjectCache.cpp index ce8757e..08def9f 100644 --- a/third_party/WebKit/Source/core/dom/AXObjectCache.cpp +++ b/third_party/WebKit/Source/core/dom/AXObjectCache.cpp
@@ -29,8 +29,14 @@ #include "core/dom/AXObjectCache.h" #include <memory> +#include "core/HTMLElementTypeHelpers.h" +#include "core/dom/Element.h" +#include "core/dom/Node.h" #include "platform/wtf/Assertions.h" +#include "platform/wtf/HashSet.h" #include "platform/wtf/PtrUtil.h" +#include "platform/wtf/text/StringHash.h" +#include "platform/wtf/text/WTFString.h" #include "public/web/WebAXEnums.h" namespace blink { @@ -76,6 +82,89 @@ return cache; } +namespace { + +typedef HashSet<String, CaseFoldingHash> ARIAWidgetSet; + +const char* g_aria_widgets[] = { + // From http://www.w3.org/TR/wai-aria/roles#widget_roles + "alert", "alertdialog", "button", "checkbox", "dialog", "gridcell", "link", + "log", "marquee", "menuitem", "menuitemcheckbox", "menuitemradio", "option", + "progressbar", "radio", "scrollbar", "slider", "spinbutton", "status", + "tab", "tabpanel", "textbox", "timer", "tooltip", "treeitem", + // Composite user interface widgets. + // This list is also from the w3.org site referenced above. + "combobox", "grid", "listbox", "menu", "menubar", "radiogroup", "tablist", + "tree", "treegrid"}; + +static ARIAWidgetSet* CreateARIARoleWidgetSet() { + ARIAWidgetSet* widget_set = new HashSet<String, CaseFoldingHash>(); + for (size_t i = 0; i < WTF_ARRAY_LENGTH(g_aria_widgets); ++i) + widget_set->insert(String(g_aria_widgets[i])); + return widget_set; +} + +bool IncludesARIAWidgetRole(const String& role) { + static const HashSet<String, CaseFoldingHash>* role_set = + CreateARIARoleWidgetSet(); + + Vector<String> role_vector; + role.Split(' ', role_vector); + for (const auto& child : role_vector) { + if (role_set->Contains(child)) + return true; + } + return false; +} + +const char* g_aria_interactive_widget_attributes[] = { + // These attributes implicitly indicate the given widget is interactive. + // From http://www.w3.org/TR/wai-aria/states_and_properties#attrs_widgets + // clang-format off + "aria-activedescendant", + "aria-checked", + "aria-controls", + "aria-disabled", // If it's disabled, it can be made interactive. + "aria-expanded", + "aria-haspopup", + "aria-multiselectable", + "aria-pressed", + "aria-required", + "aria-selected" + // clang-format on +}; + +bool HasInteractiveARIAAttribute(const Element& element) { + for (size_t i = 0; i < WTF_ARRAY_LENGTH(g_aria_interactive_widget_attributes); + ++i) { + const char* attribute = g_aria_interactive_widget_attributes[i]; + if (element.hasAttribute(attribute)) { + return true; + } + } + return false; +} + +} // namespace + +bool AXObjectCache::IsInsideFocusableElementOrARIAWidget(const Node& node) { + const Node* cur_node = &node; + do { + if (cur_node->IsElementNode()) { + const Element* element = ToElement(cur_node); + if (element->IsFocusable()) + return true; + String role = element->getAttribute("role"); + if (!role.IsEmpty() && IncludesARIAWidgetRole(role)) + return true; + if (HasInteractiveARIAAttribute(*element)) + return true; + } + cur_node = cur_node->parentNode(); + } while (cur_node && !isHTMLBodyElement(node)); + return false; +} + STATIC_ASSERT_ENUM(kWebAXEventActiveDescendantChanged, AXObjectCache::kAXActiveDescendantChanged); STATIC_ASSERT_ENUM(kWebAXEventAlert, AXObjectCache::kAXAlert);
diff --git a/third_party/WebKit/Source/core/dom/AXObjectCache.h b/third_party/WebKit/Source/core/dom/AXObjectCache.h index dad591df..e4d3f8d3 100644 --- a/third_party/WebKit/Source/core/dom/AXObjectCache.h +++ b/third_party/WebKit/Source/core/dom/AXObjectCache.h
@@ -152,6 +152,9 @@ typedef AXObjectCache* (*AXObjectCacheCreateFunction)(Document&); static void Init(AXObjectCacheCreateFunction); + // Static helper functions. + static bool IsInsideFocusableElementOrARIAWidget(const Node&); + protected: AXObjectCache();
diff --git a/third_party/WebKit/Source/core/dom/BUILD.gn b/third_party/WebKit/Source/core/dom/BUILD.gn index 51e8557..3ed7332 100644 --- a/third_party/WebKit/Source/core/dom/BUILD.gn +++ b/third_party/WebKit/Source/core/dom/BUILD.gn
@@ -8,8 +8,6 @@ split_count = 5 sources = [ - "AXObject.cpp", - "AXObject.h", "AXObjectCache.cpp", "AXObjectCache.h", "AXObjectCacheBase.cpp",
diff --git a/third_party/WebKit/Source/core/dom/DOMImplementation.cpp b/third_party/WebKit/Source/core/dom/DOMImplementation.cpp index 6eed2f0..63d8b98 100644 --- a/third_party/WebKit/Source/core/dom/DOMImplementation.cpp +++ b/third_party/WebKit/Source/core/dom/DOMImplementation.cpp
@@ -113,9 +113,9 @@ } bool DOMImplementation::IsXMLMIMEType(const String& mime_type) { - if (DeprecatedEqualIgnoringCase(mime_type, "text/xml") || - DeprecatedEqualIgnoringCase(mime_type, "application/xml") || - DeprecatedEqualIgnoringCase(mime_type, "text/xsl")) + if (EqualIgnoringASCIICase(mime_type, "text/xml") || + EqualIgnoringASCIICase(mime_type, "application/xml") || + EqualIgnoringASCIICase(mime_type, "text/xsl")) return true; // Per RFCs 3023 and 2045, an XML MIME type is of the form: @@ -191,9 +191,9 @@ static bool IsTextPlainType(const String& mime_type) { return mime_type.StartsWithIgnoringASCIICase("text/") && - !(DeprecatedEqualIgnoringCase(mime_type, "text/html") || - DeprecatedEqualIgnoringCase(mime_type, "text/xml") || - DeprecatedEqualIgnoringCase(mime_type, "text/xsl")); + !(EqualIgnoringASCIICase(mime_type, "text/html") || + EqualIgnoringASCIICase(mime_type, "text/xml") || + EqualIgnoringASCIICase(mime_type, "text/xsl")); } bool DOMImplementation::IsTextMIMEType(const String& mime_type) {
diff --git a/third_party/WebKit/Source/core/dom/ModulatorImpl.cpp b/third_party/WebKit/Source/core/dom/ModulatorImpl.cpp index bc3ff25..1867afb 100644 --- a/third_party/WebKit/Source/core/dom/ModulatorImpl.cpp +++ b/third_party/WebKit/Source/core/dom/ModulatorImpl.cpp
@@ -171,20 +171,21 @@ // https://crbug.com/715376 CHECK(RuntimeEnabledFeatures::ModuleScriptsEnabled()); - // 1. "Let settings be the settings object of s." + // Step 1. "Let settings be the settings object of s." [spec text] // The settings object is |this|. - // 2. "Check if we can run script with settings. - // If this returns "do not run" then abort these steps." + // Step 2. "Check if we can run script with settings. + // If this returns "do not run" then abort these steps." [spec text] if (!GetExecutionContext()->CanExecuteScripts(kAboutToExecuteScript)) return; - // 6. "Prepare to run script given settings." + // Step 4. "Prepare to run script given settings." [spec text] // This is placed here to also cover ScriptModule::ReportException(). ScriptState::Scope scope(script_state_.Get()); - // 3. "If s's instantiation state is "errored", then report the exception - // given by s's instantiation error for s and abort these steps." + // Step 3. "If s is errored, then report the exception given by s's error for + // s and abort these steps." [spec text] + // TODO(kouhei): Update "is errored". ModuleInstantiationState instantiationState = module_script->State(); if (instantiationState == ModuleInstantiationState::kErrored) { v8::Isolate* isolate = script_state_->GetIsolate(); @@ -194,18 +195,18 @@ return; } - // 4. "Assert: s's instantiation state is "instantiated" (and thus its - // module record is not null)." - CHECK_EQ(instantiationState, ModuleInstantiationState::kInstantiated); - - // 5. "Let record be s's module record." + // Step 5. "Let record be s's module record." [spec text] const ScriptModule& record = module_script->Record(); CHECK(!record.IsNull()); - // Steps 7 and 8. + // Step 6. "Let evaluationStatus be record.ModuleEvaluation()." [spec text] record.Evaluate(script_state_.Get()); - // 9. "Clean up after running script with settings." + // Step 7. "If evaluationStatus is an abrupt completion, then report the + // exception given by evaluationStatus.[[Value]] for s." [spec text] + // TODO(kouhei): Implement this. + + // Step 8. "Clean up after running script with settings." [spec text] // Implemented as the ScriptState::Scope destructor. }
diff --git a/third_party/WebKit/Source/core/dom/RemoteSecurityContext.cpp b/third_party/WebKit/Source/core/dom/RemoteSecurityContext.cpp index 1ee43b7d..693f947 100644 --- a/third_party/WebKit/Source/core/dom/RemoteSecurityContext.cpp +++ b/third_party/WebKit/Source/core/dom/RemoteSecurityContext.cpp
@@ -31,8 +31,7 @@ SecurityContext::Trace(visitor); } -void RemoteSecurityContext::SetReplicatedOrigin( - PassRefPtr<SecurityOrigin> origin) { +void RemoteSecurityContext::SetReplicatedOrigin(RefPtr<SecurityOrigin> origin) { DCHECK(origin); SetSecurityOrigin(std::move(origin)); GetContentSecurityPolicy()->SetupSelf(*GetSecurityOrigin());
diff --git a/third_party/WebKit/Source/core/dom/RemoteSecurityContext.h b/third_party/WebKit/Source/core/dom/RemoteSecurityContext.h index ce48cc6..8bed611 100644 --- a/third_party/WebKit/Source/core/dom/RemoteSecurityContext.h +++ b/third_party/WebKit/Source/core/dom/RemoteSecurityContext.h
@@ -20,7 +20,7 @@ DECLARE_VIRTUAL_TRACE(); static RemoteSecurityContext* Create(); - void SetReplicatedOrigin(PassRefPtr<SecurityOrigin>); + void SetReplicatedOrigin(RefPtr<SecurityOrigin>); void ResetReplicatedContentSecurityPolicy(); // FIXME: implement
diff --git a/third_party/WebKit/Source/core/editing/VisibleUnitsLine.cpp b/third_party/WebKit/Source/core/editing/VisibleUnitsLine.cpp index 5a820f3..215ab70 100644 --- a/third_party/WebKit/Source/core/editing/VisibleUnitsLine.cpp +++ b/third_party/WebKit/Source/core/editing/VisibleUnitsLine.cpp
@@ -85,6 +85,14 @@ return HighestEditableRoot(position); } +ContainerNode* HighestEditableRootOfNode(const Node& node, + EditableType editable_type) { + // TODO(editing-dev): We should introduce |const Node&| version of + // |FirstPositionInOrBeforeNode()|. See http://crbug.com/734849 + return HighestEditableRoot( + FirstPositionInOrBeforeNode(const_cast<Node*>(&node)), editable_type); +} + Node* PreviousNodeConsideringAtomicNodes(const Node& start) { if (start.previousSibling()) { Node* node = start.previousSibling(); @@ -288,8 +296,7 @@ FindNodeInPreviousLine(*node, visible_position, editable_type); for (Node* runner = previous_node; runner && !runner->IsShadowRoot(); runner = PreviousLeafWithSameEditability(runner, editable_type)) { - if (HighestEditableRoot(FirstPositionInOrBeforeNode(runner), - editable_type) != highest_root) + if (HighestEditableRootOfNode(*runner, editable_type) != highest_root) break; const Position& candidate = @@ -315,8 +322,7 @@ for (Node* runner = next_node; runner && !runner->IsShadowRoot(); runner = NextLeafWithSameEditability(runner, editable_type)) { - if (HighestEditableRoot(FirstPositionInOrBeforeNode(runner), - editable_type) != highest_root) + if (HighestEditableRootOfNode(*runner, editable_type) != highest_root) break; const Position& candidate =
diff --git a/third_party/WebKit/Source/core/exported/WebNode.cpp b/third_party/WebKit/Source/core/exported/WebNode.cpp index 2341b2b..fa557c2e 100644 --- a/third_party/WebKit/Source/core/exported/WebNode.cpp +++ b/third_party/WebKit/Source/core/exported/WebNode.cpp
@@ -31,7 +31,7 @@ #include "public/web/WebNode.h" #include "bindings/core/v8/ExceptionState.h" -#include "core/dom/AXObject.h" +#include "core/dom/AXObjectCache.h" #include "core/dom/Document.h" #include "core/dom/Element.h" #include "core/dom/Node.h" @@ -144,7 +144,7 @@ } bool WebNode::IsInsideFocusableElementOrARIAWidget() const { - return AXObject::IsInsideFocusableElementOrARIAWidget( + return AXObjectCache::IsInsideFocusableElementOrARIAWidget( *this->ConstUnwrap<Node>()); }
diff --git a/third_party/WebKit/Source/core/exported/WebSecurityPolicy.cpp b/third_party/WebKit/Source/core/exported/WebSecurityPolicy.cpp index 71c2537..991a64a 100644 --- a/third_party/WebKit/Source/core/exported/WebSecurityPolicy.cpp +++ b/third_party/WebKit/Source/core/exported/WebSecurityPolicy.cpp
@@ -86,7 +86,7 @@ void WebSecurityPolicy::AddOriginTrustworthyWhiteList( const WebSecurityOrigin& origin) { - SecurityPolicy::AddOriginTrustworthyWhiteList(origin); + SecurityPolicy::AddOriginTrustworthyWhiteList(*origin.Get()); } void WebSecurityPolicy::AddSchemeToBypassSecureContextWhitelist(
diff --git a/third_party/WebKit/Source/core/layout/LayoutBox.cpp b/third_party/WebKit/Source/core/layout/LayoutBox.cpp index 1c597efd..e9d534a27 100644 --- a/third_party/WebKit/Source/core/layout/LayoutBox.cpp +++ b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
@@ -2584,6 +2584,15 @@ static bool ShouldRecalculateMinMaxWidthsAffectedByAncestor( const LayoutBox* box) { + if (box->PreferredLogicalWidthsDirty()) { + // If the preferred widths are already dirty at this point (during layout), + // it actually means that we never need to calculate them, since that should + // have been carried out by an ancestor that's sized based on preferred + // widths (a shrink-to-fit container, for instance). In such cases the + // object will be left as dirty indefinitely, and it would just be a waste + // of time to calculate the preferred withs when nobody needs them. + return false; + } if (const LayoutBox* containing_block = box->ContainingBlock()) { if (containing_block->NeedsPreferredWidthsRecalculation()) { // If our containing block also has min/max widths that are affected by
diff --git a/third_party/WebKit/Source/core/style/BUILD.gn b/third_party/WebKit/Source/core/style/BUILD.gn index 69a18f5..98b0a62 100644 --- a/third_party/WebKit/Source/core/style/BUILD.gn +++ b/third_party/WebKit/Source/core/style/BUILD.gn
@@ -79,8 +79,6 @@ "StyleInheritedVariables.cpp", "StyleInheritedVariables.h", "StyleInvalidImage.h", - "StyleMotionData.cpp", - "StyleMotionData.h", "StyleNonInheritedVariables.cpp", "StyleNonInheritedVariables.h", "StyleOffsetRotation.h",
diff --git a/third_party/WebKit/Source/core/style/ComputedStyle.cpp b/third_party/WebKit/Source/core/style/ComputedStyle.cpp index 4b75a33..390ad8c 100644 --- a/third_party/WebKit/Source/core/style/ComputedStyle.cpp +++ b/third_party/WebKit/Source/core/style/ComputedStyle.cpp
@@ -988,42 +988,43 @@ float origin_y, const FloatRect& bounding_box, TransformationMatrix& transform) const { - const StyleMotionData& motion_data = - rare_non_inherited_data_->transform_data_->motion_; // TODO(ericwilligers): crbug.com/638055 Apply offset-position. - if (!motion_data.path_) { + if (!OffsetPath()) { return; } const LengthPoint& position = OffsetPosition(); const LengthPoint& anchor = OffsetAnchor(); + const Length& distance = OffsetDistance(); + const BasicShape* path = OffsetPath(); + const StyleOffsetRotation& rotate = OffsetRotate(); FloatPoint point; float angle; - if (motion_data.path_->GetType() == BasicShape::kStyleRayType) { + if (path->GetType() == BasicShape::kStyleRayType) { // TODO(ericwilligers): crbug.com/641245 Support <size> for ray paths. - float distance = FloatValueForLength(motion_data.distance_, 0); + float float_distance = FloatValueForLength(distance, 0); - angle = ToStyleRay(*motion_data.path_).Angle() - 90; - point.SetX(distance * cos(deg2rad(angle))); - point.SetY(distance * sin(deg2rad(angle))); + angle = ToStyleRay(*path).Angle() - 90; + point.SetX(float_distance * cos(deg2rad(angle))); + point.SetY(float_distance * sin(deg2rad(angle))); } else { - const StylePath& motion_path = ToStylePath(*motion_data.path_); + const StylePath& motion_path = ToStylePath(*path); float path_length = motion_path.length(); - float distance = FloatValueForLength(motion_data.distance_, path_length); + float float_distance = FloatValueForLength(distance, path_length); float computed_distance; if (motion_path.IsClosed() && path_length > 0) { - computed_distance = fmod(distance, path_length); + computed_distance = fmod(float_distance, path_length); if (computed_distance < 0) computed_distance += path_length; } else { - computed_distance = clampTo<float>(distance, 0, path_length); + computed_distance = clampTo<float>(float_distance, 0, path_length); } motion_path.GetPath().PointAndNormalAtLength(computed_distance, point, angle); } - if (motion_data.rotation_.type == kOffsetRotationFixed) + if (rotate.type == kOffsetRotationFixed) angle = 0; float origin_shift_x = 0; @@ -1042,7 +1043,7 @@ transform.Translate(point.X() - origin_x + origin_shift_x, point.Y() - origin_y + origin_shift_y); - transform.Rotate(angle + motion_data.rotation_.angle); + transform.Rotate(angle + rotate.angle); if (position.X() != Length(kAuto) || anchor.X() != Length(kAuto)) // Shift the origin back to transform-origin. @@ -2043,7 +2044,7 @@ } void ComputedStyle::SetOffsetPath(RefPtr<BasicShape> path) { - rare_non_inherited_data_.Access()->transform_data_.Access()->motion_.path_ = + rare_non_inherited_data_.Access()->transform_data_.Access()->offset_path_ = std::move(path); }
diff --git a/third_party/WebKit/Source/core/style/ComputedStyle.h b/third_party/WebKit/Source/core/style/ComputedStyle.h index c69c2eb..96c93d9 100644 --- a/third_party/WebKit/Source/core/style/ComputedStyle.h +++ b/third_party/WebKit/Source/core/style/ComputedStyle.h
@@ -433,19 +433,6 @@ SET_NESTED_VAR(rare_non_inherited_data_, filter_, operations_, ops); } - // backface-visibility (aka -webkit-backface-visibility) - static EBackfaceVisibility InitialBackfaceVisibility() { - return EBackfaceVisibility::kVisible; - } - EBackfaceVisibility BackfaceVisibility() const { - return static_cast<EBackfaceVisibility>( - rare_non_inherited_data_->backface_visibility_); - } - void SetBackfaceVisibility(EBackfaceVisibility b) { - SET_VAR(rare_non_inherited_data_, backface_visibility_, - static_cast<unsigned>(b)); - } - // Background properties. // background-color static Color InitialBackgroundColor() { return Color::kTransparent; } @@ -793,59 +780,13 @@ SET_NESTED_VAR(rare_non_inherited_data_, grid_data_, grid_auto_flow_, flow); } - // offset-anchor - static LengthPoint InitialOffsetAnchor() { - return LengthPoint(Length(kAuto), Length(kAuto)); - } - const LengthPoint& OffsetAnchor() const { - return rare_non_inherited_data_->transform_data_->motion_.anchor_; - } - void SetOffsetAnchor(const LengthPoint& offset_anchor) { - SET_NESTED_VAR(rare_non_inherited_data_, transform_data_, motion_.anchor_, - offset_anchor); - } - - // offset-distance - static Length InitialOffsetDistance() { return Length(0, kFixed); } - const Length& OffsetDistance() const { - return rare_non_inherited_data_->transform_data_->motion_.distance_; - } - void SetOffsetDistance(const Length& offset_distance) { - SET_NESTED_VAR(rare_non_inherited_data_, transform_data_, motion_.distance_, - offset_distance); - } - // offset-path static BasicShape* InitialOffsetPath() { return nullptr; } BasicShape* OffsetPath() const { - return rare_non_inherited_data_->transform_data_->motion_.path_.Get(); + return rare_non_inherited_data_->transform_data_->offset_path_.Get(); } void SetOffsetPath(RefPtr<BasicShape>); - // offset-position - static LengthPoint InitialOffsetPosition() { - return LengthPoint(Length(kAuto), Length(kAuto)); - } - const LengthPoint& OffsetPosition() const { - return rare_non_inherited_data_->transform_data_->motion_.position_; - } - void SetOffsetPosition(const LengthPoint& offset_position) { - SET_NESTED_VAR(rare_non_inherited_data_, transform_data_, motion_.position_, - offset_position); - } - - // offset-rotate - static StyleOffsetRotation InitialOffsetRotate() { - return StyleOffsetRotation(0, kOffsetRotationAuto); - } - const StyleOffsetRotation& OffsetRotate() const { - return rare_non_inherited_data_->transform_data_->motion_.rotation_; - } - void SetOffsetRotate(const StyleOffsetRotation& offset_rotate) { - SET_NESTED_VAR(rare_non_inherited_data_, transform_data_, motion_.rotation_, - offset_rotate); - } - // opacity (aka -webkit-opacity) static float InitialOpacity() { return 1.0f; } float Opacity() const { return rare_non_inherited_data_->opacity_; }
diff --git a/third_party/WebKit/Source/core/style/ComputedStyleConstants.h b/third_party/WebKit/Source/core/style/ComputedStyleConstants.h index b04b54f..6939201 100644 --- a/third_party/WebKit/Source/core/style/ComputedStyleConstants.h +++ b/third_party/WebKit/Source/core/style/ComputedStyleConstants.h
@@ -194,8 +194,6 @@ enum OffsetRotationType { kOffsetRotationAuto, kOffsetRotationFixed }; -enum class EBackfaceVisibility { kVisible, kHidden }; - enum ELineClampType { kLineClampLineCount, kLineClampPercentage }; enum class TextEmphasisMark {
diff --git a/third_party/WebKit/Source/core/style/StyleMotionData.cpp b/third_party/WebKit/Source/core/style/StyleMotionData.cpp deleted file mode 100644 index df6f3cf8..0000000 --- a/third_party/WebKit/Source/core/style/StyleMotionData.cpp +++ /dev/null
@@ -1,19 +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. - -#include "core/style/StyleMotionData.h" - -#include "core/style/DataEquivalency.h" - -namespace blink { - -bool StyleMotionData::operator==(const StyleMotionData& o) const { - if (anchor_ != o.anchor_ || position_ != o.position_ || - distance_ != o.distance_ || rotation_ != o.rotation_) - return false; - - return DataEquivalent(path_, o.path_); -} - -} // namespace blink
diff --git a/third_party/WebKit/Source/core/style/StyleMotionData.h b/third_party/WebKit/Source/core/style/StyleMotionData.h deleted file mode 100644 index b284a8c..0000000 --- a/third_party/WebKit/Source/core/style/StyleMotionData.h +++ /dev/null
@@ -1,45 +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 StyleMotionData_h -#define StyleMotionData_h - -#include "core/style/StyleOffsetRotation.h" -#include "core/style/StylePath.h" -#include "platform/Length.h" -#include "platform/LengthPoint.h" -#include "platform/wtf/Allocator.h" - -namespace blink { - -class StyleMotionData { - DISALLOW_NEW(); - - public: - StyleMotionData(const LengthPoint& anchor, - const LengthPoint& position, - BasicShape* path, - const Length& distance, - StyleOffsetRotation rotation) - : anchor_(anchor), - position_(position), - path_(path), - distance_(distance), - rotation_(rotation) {} - - bool operator==(const StyleMotionData&) const; - - bool operator!=(const StyleMotionData& o) const { return !(*this == o); } - - // Must be public for SET_VAR in ComputedStyle.h - LengthPoint anchor_; - LengthPoint position_; - RefPtr<BasicShape> path_; // nullptr indicates path is 'none' - Length distance_; - StyleOffsetRotation rotation_; -}; - -} // namespace blink - -#endif // StyleMotionData_h
diff --git a/third_party/WebKit/Source/core/workers/WorkerThread.cpp b/third_party/WebKit/Source/core/workers/WorkerThread.cpp index 2a16f04..394e31a 100644 --- a/third_party/WebKit/Source/core/workers/WorkerThread.cpp +++ b/third_party/WebKit/Source/core/workers/WorkerThread.cpp
@@ -58,6 +58,7 @@ #include "platform/wtf/PtrUtil.h" #include "platform/wtf/Threading.h" #include "platform/wtf/text/WTFString.h" +#include "public/platform/Platform.h" namespace blink { @@ -293,6 +294,14 @@ return false; } +InterfaceProvider* WorkerThread::GetInterfaceProvider() { + // TODO(https://crbug.com/734210): Instead of returning this interface + // provider, which maps to a RenderProcessHost in the browser process, this + // method should return an interface provider which maps to a specific worker + // context such as a SharedWorkerHost or EmbeddedWorkerInstance. + return Platform::Current()->GetInterfaceProvider(); +} + WorkerThread::WorkerThread(ThreadableLoadingContext* loading_context, WorkerReportingProxy& worker_reporting_proxy) : worker_thread_id_(GetNextWorkerThreadId()),
diff --git a/third_party/WebKit/Source/core/workers/WorkerThread.h b/third_party/WebKit/Source/core/workers/WorkerThread.h index 8de8cb3..d91f4483 100644 --- a/third_party/WebKit/Source/core/workers/WorkerThread.h +++ b/third_party/WebKit/Source/core/workers/WorkerThread.h
@@ -48,6 +48,7 @@ class ConsoleMessageStorage; class InspectorTaskRunner; +class InterfaceProvider; class WorkerBackingThread; class WorkerInspectorController; class WorkerOrWorkletGlobalScope; @@ -175,6 +176,8 @@ return global_scope_scheduler_.get(); } + InterfaceProvider* GetInterfaceProvider(); + protected: WorkerThread(ThreadableLoadingContext*, WorkerReportingProxy&);
diff --git a/third_party/WebKit/Source/core/dom/AXObject.cpp b/third_party/WebKit/Source/modules/accessibility/AXObject.cpp similarity index 83% rename from third_party/WebKit/Source/core/dom/AXObject.cpp rename to third_party/WebKit/Source/modules/accessibility/AXObject.cpp index b0252e1..b814e741 100644 --- a/third_party/WebKit/Source/core/dom/AXObject.cpp +++ b/third_party/WebKit/Source/modules/accessibility/AXObject.cpp
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "core/dom/AXObject.h" +#include "modules/accessibility/AXObject.h" #include "core/HTMLElementTypeHelpers.h" #include "core/dom/Element.h" @@ -15,89 +15,6 @@ namespace blink { -namespace { - -typedef HashSet<String, CaseFoldingHash> ARIAWidgetSet; - -const char* g_aria_widgets[] = { - // From http://www.w3.org/TR/wai-aria/roles#widget_roles - "alert", "alertdialog", "button", "checkbox", "dialog", "gridcell", "link", - "log", "marquee", "menuitem", "menuitemcheckbox", "menuitemradio", "option", - "progressbar", "radio", "scrollbar", "slider", "spinbutton", "status", - "tab", "tabpanel", "textbox", "timer", "tooltip", "treeitem", - // Composite user interface widgets. - // This list is also from the w3.org site referenced above. - "combobox", "grid", "listbox", "menu", "menubar", "radiogroup", "tablist", - "tree", "treegrid"}; - -static ARIAWidgetSet* CreateARIARoleWidgetSet() { - ARIAWidgetSet* widget_set = new HashSet<String, CaseFoldingHash>(); - for (size_t i = 0; i < WTF_ARRAY_LENGTH(g_aria_widgets); ++i) - widget_set->insert(String(g_aria_widgets[i])); - return widget_set; -} - -bool IncludesARIAWidgetRole(const String& role) { - static const HashSet<String, CaseFoldingHash>* role_set = - CreateARIARoleWidgetSet(); - - Vector<String> role_vector; - role.Split(' ', role_vector); - for (const auto& child : role_vector) { - if (role_set->Contains(child)) - return true; - } - return false; -} - -const char* g_aria_interactive_widget_attributes[] = { - // These attributes implicitly indicate the given widget is interactive. - // From http://www.w3.org/TR/wai-aria/states_and_properties#attrs_widgets - // clang-format off - "aria-activedescendant", - "aria-checked", - "aria-controls", - "aria-disabled", // If it's disabled, it can be made interactive. - "aria-expanded", - "aria-haspopup", - "aria-multiselectable", - "aria-pressed", - "aria-required", - "aria-selected" - // clang-format on -}; - -bool HasInteractiveARIAAttribute(const Element& element) { - for (size_t i = 0; i < WTF_ARRAY_LENGTH(g_aria_interactive_widget_attributes); - ++i) { - const char* attribute = g_aria_interactive_widget_attributes[i]; - if (element.hasAttribute(attribute)) { - return true; - } - } - return false; -} - -} // namespace - -bool AXObject::IsInsideFocusableElementOrARIAWidget(const Node& node) { - const Node* cur_node = &node; - do { - if (cur_node->IsElementNode()) { - const Element* element = ToElement(cur_node); - if (element->IsFocusable()) - return true; - String role = element->getAttribute("role"); - if (!role.IsEmpty() && IncludesARIAWidgetRole(role)) - return true; - if (HasInteractiveARIAAttribute(*element)) - return true; - } - cur_node = cur_node->parentNode(); - } while (cur_node && !isHTMLBodyElement(node)); - return false; -} - STATIC_ASSERT_ENUM(kWebAXRoleAbbr, kAbbrRole); STATIC_ASSERT_ENUM(kWebAXRoleAlertDialog, kAlertDialogRole); STATIC_ASSERT_ENUM(kWebAXRoleAlert, kAlertRole);
diff --git a/third_party/WebKit/Source/core/dom/AXObject.h b/third_party/WebKit/Source/modules/accessibility/AXObject.h similarity index 97% rename from third_party/WebKit/Source/core/dom/AXObject.h rename to third_party/WebKit/Source/modules/accessibility/AXObject.h index 8760067..5d37161 100644 --- a/third_party/WebKit/Source/core/dom/AXObject.h +++ b/third_party/WebKit/Source/modules/accessibility/AXObject.h
@@ -9,8 +9,6 @@ namespace blink { -class Node; - enum AccessibilityRole { kUnknownRole = 0, // Not mapped in platform APIs, generally indicates a bug kAbbrRole, // No mapping to ARIA role. @@ -284,11 +282,7 @@ // TODO(sashab): Add pure virtual methods to this class to remove dependencies // on AXObjectImpl from outside of modules/. -class CORE_EXPORT AXObject { - public: - // Static helper functions. - static bool IsInsideFocusableElementOrARIAWidget(const Node&); -}; +class CORE_EXPORT AXObject {}; } // namespace blink
diff --git a/third_party/WebKit/Source/modules/accessibility/AXObjectImpl.h b/third_party/WebKit/Source/modules/accessibility/AXObjectImpl.h index a2a9acc8..c2487266 100644 --- a/third_party/WebKit/Source/modules/accessibility/AXObjectImpl.h +++ b/third_party/WebKit/Source/modules/accessibility/AXObjectImpl.h
@@ -31,11 +31,11 @@ #ifndef AXObjectImpl_h #define AXObjectImpl_h -#include "core/dom/AXObject.h" #include "core/editing/VisiblePosition.h" #include "core/editing/markers/DocumentMarker.h" #include "core/inspector/protocol/Accessibility.h" #include "modules/ModulesExport.h" +#include "modules/accessibility/AXObject.h" #include "platform/geometry/FloatQuad.h" #include "platform/geometry/LayoutRect.h" #include "platform/graphics/Color.h"
diff --git a/third_party/WebKit/Source/modules/accessibility/AXObjectTest.cpp b/third_party/WebKit/Source/modules/accessibility/AXObjectTest.cpp index 7a71cb0..b0e27de 100644 --- a/third_party/WebKit/Source/modules/accessibility/AXObjectTest.cpp +++ b/third_party/WebKit/Source/modules/accessibility/AXObjectTest.cpp
@@ -48,25 +48,25 @@ GetDocument().documentElement()->setInnerHTML(test_content); GetDocument().UpdateStyleAndLayout(); Element* root(GetDocument().documentElement()); - EXPECT_FALSE(AXObject::IsInsideFocusableElementOrARIAWidget( + EXPECT_FALSE(AXObjectCache::IsInsideFocusableElementOrARIAWidget( *root->getElementById("plain"))); - EXPECT_TRUE(AXObject::IsInsideFocusableElementOrARIAWidget( + EXPECT_TRUE(AXObjectCache::IsInsideFocusableElementOrARIAWidget( *root->getElementById("button"))); - EXPECT_TRUE(AXObject::IsInsideFocusableElementOrARIAWidget( + EXPECT_TRUE(AXObjectCache::IsInsideFocusableElementOrARIAWidget( *root->getElementById("button-parent"))); - EXPECT_TRUE(AXObject::IsInsideFocusableElementOrARIAWidget( + EXPECT_TRUE(AXObjectCache::IsInsideFocusableElementOrARIAWidget( *root->getElementById("button-caps"))); - EXPECT_TRUE(AXObject::IsInsideFocusableElementOrARIAWidget( + EXPECT_TRUE(AXObjectCache::IsInsideFocusableElementOrARIAWidget( *root->getElementById("button-second"))); - EXPECT_FALSE(AXObject::IsInsideFocusableElementOrARIAWidget( + EXPECT_FALSE(AXObjectCache::IsInsideFocusableElementOrARIAWidget( *root->getElementById("aria-bogus"))); - EXPECT_TRUE(AXObject::IsInsideFocusableElementOrARIAWidget( + EXPECT_TRUE(AXObjectCache::IsInsideFocusableElementOrARIAWidget( *root->getElementById("aria-selected"))); - EXPECT_TRUE(AXObject::IsInsideFocusableElementOrARIAWidget( + EXPECT_TRUE(AXObjectCache::IsInsideFocusableElementOrARIAWidget( *root->getElementById("haspopup"))); - EXPECT_TRUE(AXObject::IsInsideFocusableElementOrARIAWidget( + EXPECT_TRUE(AXObjectCache::IsInsideFocusableElementOrARIAWidget( *root->getElementById("focusable"))); - EXPECT_TRUE(AXObject::IsInsideFocusableElementOrARIAWidget( + EXPECT_TRUE(AXObjectCache::IsInsideFocusableElementOrARIAWidget( *root->getElementById("focusable-parent"))); }
diff --git a/third_party/WebKit/Source/modules/accessibility/BUILD.gn b/third_party/WebKit/Source/modules/accessibility/BUILD.gn index 44b6d1b..80d1b04 100644 --- a/third_party/WebKit/Source/modules/accessibility/BUILD.gn +++ b/third_party/WebKit/Source/modules/accessibility/BUILD.gn
@@ -36,6 +36,8 @@ "AXMockObject.h", "AXNodeObject.cpp", "AXNodeObject.h", + "AXObject.cpp", + "AXObject.h", "AXObjectCacheImpl.cpp", "AXObjectCacheImpl.h", "AXObjectImpl.cpp",
diff --git a/third_party/WebKit/Source/modules/permissions/DEPS b/third_party/WebKit/Source/modules/permissions/DEPS index 16764a92..dadedbdc 100644 --- a/third_party/WebKit/Source/modules/permissions/DEPS +++ b/third_party/WebKit/Source/modules/permissions/DEPS
@@ -1,3 +1,4 @@ include_rules = [ "+mojo/public/cpp/bindings", + "+services/service_manager/public/cpp", ]
diff --git a/third_party/WebKit/Source/modules/permissions/PermissionUtils.cpp b/third_party/WebKit/Source/modules/permissions/PermissionUtils.cpp index 6f66438..d6a77fb 100644 --- a/third_party/WebKit/Source/modules/permissions/PermissionUtils.cpp +++ b/third_party/WebKit/Source/modules/permissions/PermissionUtils.cpp
@@ -7,8 +7,11 @@ #include "core/dom/Document.h" #include "core/dom/ExecutionContext.h" #include "core/frame/LocalFrame.h" +#include "core/frame/LocalFrameClient.h" +#include "core/workers/WorkerGlobalScope.h" +#include "core/workers/WorkerThread.h" #include "public/platform/InterfaceProvider.h" -#include "public/platform/Platform.h" +#include "services/service_manager/public/cpp/interface_provider.h" namespace blink { @@ -19,18 +22,21 @@ bool ConnectToPermissionService( ExecutionContext* execution_context, mojom::blink::PermissionServiceRequest request) { - InterfaceProvider* interface_provider = nullptr; if (execution_context->IsDocument()) { - Document* document = ToDocument(execution_context); - if (document->GetFrame()) - interface_provider = document->GetFrame()->GetInterfaceProvider(); - } else { - interface_provider = Platform::Current()->GetInterfaceProvider(); + LocalFrame* frame = ToDocument(execution_context)->GetFrame(); + if (frame) { + frame->Client()->GetInterfaceProvider()->GetInterface(std::move(request)); + return true; + } + } else if (execution_context->IsWorkerGlobalScope()) { + WorkerThread* thread = ToWorkerGlobalScope(execution_context)->GetThread(); + if (thread) { + thread->GetInterfaceProvider()->GetInterface(std::move(request)); + return true; + } } - if (interface_provider) - interface_provider->GetInterface(std::move(request)); - return interface_provider; + return false; } PermissionDescriptorPtr CreatePermissionDescriptor(PermissionName name) {
diff --git a/third_party/WebKit/Source/modules/websockets/WebPepperSocketImpl.cpp b/third_party/WebKit/Source/modules/websockets/WebPepperSocketImpl.cpp index a401513..2c85006d3 100644 --- a/third_party/WebKit/Source/modules/websockets/WebPepperSocketImpl.cpp +++ b/third_party/WebKit/Source/modules/websockets/WebPepperSocketImpl.cpp
@@ -30,6 +30,7 @@ #include "modules/websockets/WebPepperSocketImpl.h" +#include <stddef.h> #include <memory> #include "core/dom/DOMArrayBuffer.h" #include "core/dom/Document.h" @@ -39,7 +40,6 @@ #include "modules/websockets/WebSocketChannel.h" #include "platform/wtf/text/CString.h" #include "platform/wtf/text/WTFString.h" -#include "public/platform/WebString.h" #include "public/platform/WebURL.h" #include "public/web/WebArrayBuffer.h" #include "public/web/WebDocument.h"
diff --git a/third_party/WebKit/Source/modules/websockets/WebPepperSocketImpl.h b/third_party/WebKit/Source/modules/websockets/WebPepperSocketImpl.h index 7881f0a3..6446b45 100644 --- a/third_party/WebKit/Source/modules/websockets/WebPepperSocketImpl.h +++ b/third_party/WebKit/Source/modules/websockets/WebPepperSocketImpl.h
@@ -34,8 +34,8 @@ #include <memory> #include "modules/websockets/WebSocketChannelClient.h" #include "platform/heap/Handle.h" -#include "platform/wtf/RefPtr.h" -#include "public/platform/WebCommon.h" +#include "platform/wtf/Forward.h" +#include "platform/wtf/Vector.h" #include "public/platform/WebString.h" #include "public/web/WebPepperSocket.h" #include "public/web/WebPepperSocketClient.h"
diff --git a/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp b/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp index 581e090..3181935 100644 --- a/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp +++ b/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp
@@ -130,8 +130,7 @@ void WorkerWebSocketChannel::Fail(const String& reason, MessageLevel level, std::unique_ptr<SourceLocation> location) { - if (!bridge_) - return; + DCHECK(bridge_); std::unique_ptr<SourceLocation> captured_location = SourceLocation::Capture(); if (!captured_location->IsUnknown()) {
diff --git a/third_party/WebKit/Source/platform/audio/AudioDestination.cpp b/third_party/WebKit/Source/platform/audio/AudioDestination.cpp index 39d569fd..b3075646 100644 --- a/third_party/WebKit/Source/platform/audio/AudioDestination.cpp +++ b/third_party/WebKit/Source/platform/audio/AudioDestination.cpp
@@ -56,7 +56,7 @@ AudioIOCallback& callback, unsigned number_of_output_channels, const WebAudioLatencyHint& latency_hint, - PassRefPtr<SecurityOrigin> security_origin) { + RefPtr<SecurityOrigin> security_origin) { return WTF::WrapUnique( new AudioDestination(callback, number_of_output_channels, latency_hint, std::move(security_origin))); @@ -65,7 +65,7 @@ AudioDestination::AudioDestination(AudioIOCallback& callback, unsigned number_of_output_channels, const WebAudioLatencyHint& latency_hint, - PassRefPtr<SecurityOrigin> security_origin) + RefPtr<SecurityOrigin> security_origin) : number_of_output_channels_(number_of_output_channels), is_playing_(false), fifo_(WTF::WrapUnique(
diff --git a/third_party/WebKit/Source/platform/audio/AudioDestination.h b/third_party/WebKit/Source/platform/audio/AudioDestination.h index bbf8426..a071206 100644 --- a/third_party/WebKit/Source/platform/audio/AudioDestination.h +++ b/third_party/WebKit/Source/platform/audio/AudioDestination.h
@@ -57,14 +57,14 @@ AudioDestination(AudioIOCallback&, unsigned number_of_output_channels, const WebAudioLatencyHint&, - PassRefPtr<SecurityOrigin>); + RefPtr<SecurityOrigin>); ~AudioDestination() override; static std::unique_ptr<AudioDestination> Create( AudioIOCallback&, unsigned number_of_output_channels, const WebAudioLatencyHint&, - PassRefPtr<SecurityOrigin>); + RefPtr<SecurityOrigin>); // The actual render function (WebAudioDevice::RenderCallback) isochronously // invoked by the media renderer.
diff --git a/third_party/WebKit/Source/platform/credentialmanager/PlatformFederatedCredential.cpp b/third_party/WebKit/Source/platform/credentialmanager/PlatformFederatedCredential.cpp index a88b2b2..dbd5544 100644 --- a/third_party/WebKit/Source/platform/credentialmanager/PlatformFederatedCredential.cpp +++ b/third_party/WebKit/Source/platform/credentialmanager/PlatformFederatedCredential.cpp
@@ -8,7 +8,7 @@ PlatformFederatedCredential* PlatformFederatedCredential::Create( const String& id, - PassRefPtr<SecurityOrigin> provider, + RefPtr<SecurityOrigin> provider, const String& name, const KURL& icon_url) { return new PlatformFederatedCredential(id, std::move(provider), name, @@ -17,7 +17,7 @@ PlatformFederatedCredential::PlatformFederatedCredential( const String& id, - PassRefPtr<SecurityOrigin> provider, + RefPtr<SecurityOrigin> provider, const String& name, const KURL& icon_url) : PlatformCredential(id, name, icon_url), provider_(std::move(provider)) {
diff --git a/third_party/WebKit/Source/platform/credentialmanager/PlatformFederatedCredential.h b/third_party/WebKit/Source/platform/credentialmanager/PlatformFederatedCredential.h index c12a45e..6536993 100644 --- a/third_party/WebKit/Source/platform/credentialmanager/PlatformFederatedCredential.h +++ b/third_party/WebKit/Source/platform/credentialmanager/PlatformFederatedCredential.h
@@ -17,20 +17,19 @@ WTF_MAKE_NONCOPYABLE(PlatformFederatedCredential); public: - static PlatformFederatedCredential* Create( - const String& id, - PassRefPtr<SecurityOrigin> provider, - const String& name, - const KURL& icon_url); + static PlatformFederatedCredential* Create(const String& id, + RefPtr<SecurityOrigin> provider, + const String& name, + const KURL& icon_url); ~PlatformFederatedCredential() override; - PassRefPtr<SecurityOrigin> Provider() const { return provider_; } + RefPtr<SecurityOrigin> Provider() const { return provider_; } bool IsFederated() override { return true; } private: PlatformFederatedCredential(const String& id, - PassRefPtr<SecurityOrigin> provider, + RefPtr<SecurityOrigin> provider, const String& name, const KURL& icon_url);
diff --git a/third_party/WebKit/Source/platform/exported/WebSecurityOrigin.cpp b/third_party/WebKit/Source/platform/exported/WebSecurityOrigin.cpp index 8e976523..141d00a 100644 --- a/third_party/WebKit/Source/platform/exported/WebSecurityOrigin.cpp +++ b/third_party/WebKit/Source/platform/exported/WebSecurityOrigin.cpp
@@ -32,7 +32,6 @@ #include "platform/weborigin/KURL.h" #include "platform/weborigin/SecurityOrigin.h" -#include "platform/wtf/PassRefPtr.h" #include "public/platform/WebString.h" #include "public/platform/WebURL.h" @@ -131,17 +130,17 @@ return private_->CanAccessPasswordManager(); } -WebSecurityOrigin::WebSecurityOrigin(WTF::PassRefPtr<SecurityOrigin> origin) +WebSecurityOrigin::WebSecurityOrigin(WTF::RefPtr<SecurityOrigin> origin) : private_(static_cast<WebSecurityOriginPrivate*>(origin.LeakRef())) {} WebSecurityOrigin& WebSecurityOrigin::operator=( - WTF::PassRefPtr<SecurityOrigin> origin) { + WTF::RefPtr<SecurityOrigin> origin) { Assign(static_cast<WebSecurityOriginPrivate*>(origin.LeakRef())); return *this; } -WebSecurityOrigin::operator WTF::PassRefPtr<SecurityOrigin>() const { - return PassRefPtr<SecurityOrigin>( +WebSecurityOrigin::operator WTF::RefPtr<SecurityOrigin>() const { + return RefPtr<SecurityOrigin>( const_cast<WebSecurityOriginPrivate*>(private_)); }
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceRequest.cpp b/third_party/WebKit/Source/platform/loader/fetch/ResourceRequest.cpp index 0e5082a..4c6b5c3 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/ResourceRequest.cpp +++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceRequest.cpp
@@ -224,12 +224,12 @@ first_party_for_cookies_ = first_party_for_cookies; } -PassRefPtr<SecurityOrigin> ResourceRequest::RequestorOrigin() const { +RefPtr<SecurityOrigin> ResourceRequest::RequestorOrigin() const { return requestor_origin_; } void ResourceRequest::SetRequestorOrigin( - PassRefPtr<SecurityOrigin> requestor_origin) { + RefPtr<SecurityOrigin> requestor_origin) { requestor_origin_ = std::move(requestor_origin); }
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceRequest.h b/third_party/WebKit/Source/platform/loader/fetch/ResourceRequest.h index f584471..dbbbb28 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/ResourceRequest.h +++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceRequest.h
@@ -100,8 +100,8 @@ const KURL& FirstPartyForCookies() const; void SetFirstPartyForCookies(const KURL&); - PassRefPtr<SecurityOrigin> RequestorOrigin() const; - void SetRequestorOrigin(PassRefPtr<SecurityOrigin>); + RefPtr<SecurityOrigin> RequestorOrigin() const; + void SetRequestorOrigin(RefPtr<SecurityOrigin>); const AtomicString& HttpMethod() const; void SetHTTPMethod(const AtomicString&);
diff --git a/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.cpp b/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.cpp index 3ff0ba0..ceea0d09 100644 --- a/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.cpp +++ b/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.cpp
@@ -176,14 +176,12 @@ is_unique_origin_potentially_trustworthy_( other->is_unique_origin_potentially_trustworthy_) {} -PassRefPtr<SecurityOrigin> SecurityOrigin::Create(const KURL& url) { +RefPtr<SecurityOrigin> SecurityOrigin::Create(const KURL& url) { if (RefPtr<SecurityOrigin> origin = GetOriginFromMap(url)) - return origin.Release(); + return origin; - if (ShouldTreatAsUniqueOrigin(url)) { - RefPtr<SecurityOrigin> origin = AdoptRef(new SecurityOrigin()); - return origin.Release(); - } + if (ShouldTreatAsUniqueOrigin(url)) + return AdoptRef(new SecurityOrigin()); if (ShouldUseInnerURL(url)) return AdoptRef(new SecurityOrigin(ExtractInnerURL(url))); @@ -191,13 +189,13 @@ return AdoptRef(new SecurityOrigin(url)); } -PassRefPtr<SecurityOrigin> SecurityOrigin::CreateUnique() { +RefPtr<SecurityOrigin> SecurityOrigin::CreateUnique() { RefPtr<SecurityOrigin> origin = AdoptRef(new SecurityOrigin()); DCHECK(origin->IsUnique()); - return origin.Release(); + return origin; } -PassRefPtr<SecurityOrigin> SecurityOrigin::IsolatedCopy() const { +RefPtr<SecurityOrigin> SecurityOrigin::IsolatedCopy() const { return AdoptRef(new SecurityOrigin(this)); } @@ -522,14 +520,14 @@ } } -PassRefPtr<SecurityOrigin> SecurityOrigin::CreateFromString( +RefPtr<SecurityOrigin> SecurityOrigin::CreateFromString( const String& origin_string) { return SecurityOrigin::Create(KURL(KURL(), origin_string)); } -PassRefPtr<SecurityOrigin> SecurityOrigin::Create(const String& protocol, - const String& host, - int port) { +RefPtr<SecurityOrigin> SecurityOrigin::Create(const String& protocol, + const String& host, + int port) { if (port < 0 || port > kMaxAllowedPort) return CreateUnique(); @@ -539,14 +537,14 @@ return Create(KURL(KURL(), protocol + "://" + host + port_part + "/")); } -PassRefPtr<SecurityOrigin> SecurityOrigin::Create(const String& protocol, - const String& host, - int port, - const String& suborigin) { +RefPtr<SecurityOrigin> SecurityOrigin::Create(const String& protocol, + const String& host, + int port, + const String& suborigin) { RefPtr<SecurityOrigin> origin = Create(protocol, host, port); if (!suborigin.IsEmpty()) origin->suborigin_.SetName(suborigin); - return origin.Release(); + return origin; } bool SecurityOrigin::IsSameSchemeHostPort(const SecurityOrigin* other) const {
diff --git a/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h b/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h index 6c14ed9..e2b8099 100644 --- a/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h +++ b/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h
@@ -46,17 +46,17 @@ WTF_MAKE_NONCOPYABLE(SecurityOrigin); public: - static PassRefPtr<SecurityOrigin> Create(const KURL&); - static PassRefPtr<SecurityOrigin> CreateUnique(); + static RefPtr<SecurityOrigin> Create(const KURL&); + static RefPtr<SecurityOrigin> CreateUnique(); - static PassRefPtr<SecurityOrigin> CreateFromString(const String&); - static PassRefPtr<SecurityOrigin> Create(const String& protocol, - const String& host, - int port); - static PassRefPtr<SecurityOrigin> Create(const String& protocol, - const String& host, - int port, - const String& suborigin); + static RefPtr<SecurityOrigin> CreateFromString(const String&); + static RefPtr<SecurityOrigin> Create(const String& protocol, + const String& host, + int port); + static RefPtr<SecurityOrigin> Create(const String& protocol, + const String& host, + int port, + const String& suborigin); static void SetMap(URLSecurityOriginMap*); @@ -75,7 +75,7 @@ // Create a deep copy of this SecurityOrigin. This method is useful // when marshalling a SecurityOrigin to another thread. - PassRefPtr<SecurityOrigin> IsolatedCopy() const; + RefPtr<SecurityOrigin> IsolatedCopy() const; // Set the domain property of this security origin to newDomain. This // function does not check whether newDomain is a suffix of the current
diff --git a/third_party/WebKit/Source/platform/weborigin/SecurityOriginTest.cpp b/third_party/WebKit/Source/platform/weborigin/SecurityOriginTest.cpp index a6eab50..67bc353 100644 --- a/third_party/WebKit/Source/platform/weborigin/SecurityOriginTest.cpp +++ b/third_party/WebKit/Source/platform/weborigin/SecurityOriginTest.cpp
@@ -217,7 +217,7 @@ for (const char* test : urls) { KURL url(kParsedURLString, test); EXPECT_FALSE(SecurityOrigin::IsSecure(url)); - SecurityPolicy::AddOriginTrustworthyWhiteList(SecurityOrigin::Create(url)); + SecurityPolicy::AddOriginTrustworthyWhiteList(*SecurityOrigin::Create(url)); EXPECT_TRUE(SecurityOrigin::IsSecure(url)); } }
diff --git a/third_party/WebKit/Source/platform/weborigin/SecurityPolicy.cpp b/third_party/WebKit/Source/platform/weborigin/SecurityPolicy.cpp index 0b411ea..256900c 100644 --- a/third_party/WebKit/Source/platform/weborigin/SecurityPolicy.cpp +++ b/third_party/WebKit/Source/platform/weborigin/SecurityPolicy.cpp
@@ -165,14 +165,14 @@ } void SecurityPolicy::AddOriginTrustworthyWhiteList( - PassRefPtr<SecurityOrigin> origin) { + const SecurityOrigin& origin) { #if DCHECK_IS_ON() // Must be called before we start other threads. DCHECK(WTF::IsBeforeThreadCreated()); #endif - if (origin->IsUnique()) + if (origin.IsUnique()) return; - TrustworthyOriginSet().insert(origin->ToRawString()); + TrustworthyOriginSet().insert(origin.ToRawString()); } bool SecurityPolicy::IsOriginWhiteListedTrustworthy(
diff --git a/third_party/WebKit/Source/platform/weborigin/SecurityPolicy.h b/third_party/WebKit/Source/platform/weborigin/SecurityPolicy.h index fbaf610..10ebe611 100644 --- a/third_party/WebKit/Source/platform/weborigin/SecurityPolicy.h +++ b/third_party/WebKit/Source/platform/weborigin/SecurityPolicy.h
@@ -81,7 +81,7 @@ static bool IsAccessToURLWhiteListed(const SecurityOrigin* active_origin, const KURL&); - static void AddOriginTrustworthyWhiteList(PassRefPtr<SecurityOrigin>); + static void AddOriginTrustworthyWhiteList(const SecurityOrigin&); static bool IsOriginWhiteListedTrustworthy(const SecurityOrigin&); static bool IsUrlWhiteListedTrustworthy(const KURL&);
diff --git a/third_party/WebKit/Source/platform/weborigin/SecurityPolicyTest.cpp b/third_party/WebKit/Source/platform/weborigin/SecurityPolicyTest.cpp index 8e0a598..e9aeefb 100644 --- a/third_party/WebKit/Source/platform/weborigin/SecurityPolicyTest.cpp +++ b/third_party/WebKit/Source/platform/weborigin/SecurityPolicyTest.cpp
@@ -218,7 +218,7 @@ for (const char* url : insecure_urls) { RefPtr<SecurityOrigin> origin = SecurityOrigin::CreateFromString(url); EXPECT_FALSE(origin->IsPotentiallyTrustworthy()); - SecurityPolicy::AddOriginTrustworthyWhiteList(origin); + SecurityPolicy::AddOriginTrustworthyWhiteList(*origin); EXPECT_TRUE(origin->IsPotentiallyTrustworthy()); } @@ -244,7 +244,7 @@ EXPECT_FALSE(origin1->IsPotentiallyTrustworthy()); EXPECT_FALSE(origin2->IsPotentiallyTrustworthy()); - SecurityPolicy::AddOriginTrustworthyWhiteList(origin1); + SecurityPolicy::AddOriginTrustworthyWhiteList(*origin1); EXPECT_TRUE(origin1->IsPotentiallyTrustworthy()); EXPECT_TRUE(origin2->IsPotentiallyTrustworthy()); }
diff --git a/third_party/WebKit/public/platform/WebSecurityOrigin.h b/third_party/WebKit/public/platform/WebSecurityOrigin.h index 6387783..6915c73 100644 --- a/third_party/WebKit/public/platform/WebSecurityOrigin.h +++ b/third_party/WebKit/public/platform/WebSecurityOrigin.h
@@ -35,7 +35,7 @@ #include "public/platform/WebString.h" #if INSIDE_BLINK -#include "platform/wtf/PassRefPtr.h" +#include "platform/wtf/RefPtr.h" #else #include "url/origin.h" #endif @@ -111,10 +111,10 @@ BLINK_PLATFORM_EXPORT void GrantLoadLocalResources() const; #if INSIDE_BLINK - BLINK_PLATFORM_EXPORT WebSecurityOrigin(WTF::PassRefPtr<SecurityOrigin>); + BLINK_PLATFORM_EXPORT WebSecurityOrigin(WTF::RefPtr<SecurityOrigin>); BLINK_PLATFORM_EXPORT WebSecurityOrigin& operator=( - WTF::PassRefPtr<SecurityOrigin>); - BLINK_PLATFORM_EXPORT operator WTF::PassRefPtr<SecurityOrigin>() const; + WTF::RefPtr<SecurityOrigin>); + BLINK_PLATFORM_EXPORT operator WTF::RefPtr<SecurityOrigin>() const; BLINK_PLATFORM_EXPORT SecurityOrigin* Get() const; #else // TODO(mkwst): A number of properties don't survive a round-trip
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 7fe5a56..b3245a13 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -24925,6 +24925,42 @@ </summary> </histogram> +<histogram name="ImportantFile.FileCreateError" enum="PlatformFileError"> + <owner>xaerox@yandex-team.ru</owner> + <summary> + File error happened upon temporary file creation at ImportantFileWriter. + </summary> +</histogram> + +<histogram name="ImportantFile.FileDeleteError" enum="PlatformFileError"> + <owner>xaerox@yandex-team.ru</owner> + <summary> + File error happened upon temporary file deletion at ImportantFileWriter. + </summary> +</histogram> + +<histogram name="ImportantFile.FileOpenError" enum="PlatformFileError"> + <owner>xaerox@yandex-team.ru</owner> + <summary> + File error happened upon opening temporary file at ImportantFileWriter. + </summary> +</histogram> + +<histogram name="ImportantFile.FileRenameError" enum="PlatformFileError"> + <owner>xaerox@yandex-team.ru</owner> + <summary> + File error happened upon temporary file renaming at ImportantFileWriter. + </summary> +</histogram> + +<histogram name="ImportantFile.FileWriteError" enum="PlatformFileError"> + <owner>xaerox@yandex-team.ru</owner> + <summary> + File error happened upon writing data to temporary file at + ImportantFileWriter. + </summary> +</histogram> + <histogram name="ImportantFile.TempFileFailures" enum="TempFileFailure"> <owner>rvargas@chromium.org</owner> <summary> @@ -51522,6 +51558,9 @@ <histogram name="PasswordManager.SettingsReconciliation.InitialAndFinalValues" enum="PasswordManagerPreferencesInitialAndFinalValues"> + <obsolete> + Deprecated 06/2017 + </obsolete> <owner>engedy@chromium.org</owner> <owner>melandory@chromium.org</owner> <summary> @@ -51535,6 +51574,9 @@ <histogram name="PasswordManager.SettingsReconciliation.InitialValues" enum="PasswordManagerPreferencesInitialValues"> + <obsolete> + Deprecated 06/2017 + </obsolete> <owner>engedy@chromium.org</owner> <owner>melandory@chromium.org</owner> <summary> @@ -67191,6 +67233,9 @@ </histogram> <histogram name="Search.GsaProcessMemoryPss" units="KB"> + <obsolete> + Deprecated 06/2017, no longer recorded. + </obsolete> <owner>lizeb@chromium.org</owner> <summary> On Android, when Chrome connects to a bound service exposed by GSA, the @@ -89484,6 +89529,17 @@ <affected-histogram name="PageLoad.Timing2.NavigationToFirstContentfulPaint"/> </histogram_suffixes> +<histogram_suffixes name="ImportantFileWriterSuffix" separator="."> + <suffix name="BookmarkStorage"/> + <suffix name="FeedbackReport"/> + <affected-histogram name="ImportantFile.FileCreateError"/> + <affected-histogram name="ImportantFile.FileDeleteError"/> + <affected-histogram name="ImportantFile.FileOpenError"/> + <affected-histogram name="ImportantFile.FileRenameError"/> + <affected-histogram name="ImportantFile.FileWriteError"/> + <affected-histogram name="ImportantFile.TempFileFailures"/> +</histogram_suffixes> + <histogram_suffixes name="IndexedDBLevelDBErrnoMethods" separator="."> <suffix name="NewLogger" label="ChromiumEnv::NewLogger"/> <suffix name="NewSequentialFile" label="ChromiumEnv::NewSequentialFile"/>
diff --git a/tools/perf/benchmarks/indexeddb_perf.py b/tools/perf/benchmarks/indexeddb_perf.py index e8799f5..5f4af66613 100644 --- a/tools/perf/benchmarks/indexeddb_perf.py +++ b/tools/perf/benchmarks/indexeddb_perf.py
@@ -99,6 +99,9 @@ def Name(cls): return 'storage.indexeddb_endure' + def GetExpectations(self): + return page_sets.IndexedDBEndureStoryExpectations() + @benchmark.Disabled('linux') # crbug.com/677972 @benchmark.Owner(emails=['cmumford@chromium.org']) @@ -121,3 +124,6 @@ @classmethod def ValueCanBeAddedPredicate(cls, value, is_first_result): return 'idb' in value.name + + def GetExpectations(self): + return page_sets.IndexedDBEndureStoryExpectations()
diff --git a/tools/perf/benchmarks/octane.py b/tools/perf/benchmarks/octane.py index d6de296..6419c8b 100644 --- a/tools/perf/benchmarks/octane.py +++ b/tools/perf/benchmarks/octane.py
@@ -154,3 +154,9 @@ ps, ps.base_dir, make_javascript_deterministic=False, name='http://chromium.github.io/octane/index.html?auto=1')) return ps + + def GetExpectations(self): + class StoryExpectations(story.expectations.StoryExpectations): + def SetExpectations(self): + pass # Octane not disabled. + return StoryExpectations()
diff --git a/tools/perf/benchmarks/oortonline.py b/tools/perf/benchmarks/oortonline.py index cddf023..f6a98a0 100644 --- a/tools/perf/benchmarks/oortonline.py +++ b/tools/perf/benchmarks/oortonline.py
@@ -8,6 +8,7 @@ from core import perf_benchmark from telemetry import benchmark +from telemetry import story from telemetry.page import legacy_page_test from telemetry.value import scalar from telemetry.value import improvement_direction @@ -49,6 +50,11 @@ def CreateStorySet(self, options): return page_sets.OortOnlinePageSet() + def GetExpectations(self): + class StoryExpectations(story.expectations.StoryExpectations): + def SetExpectations(self): + pass # http://oortonline.gl/#run not disabled. + return StoryExpectations() # Disabled on Linux due to timeouts; crbug.com/727850 @benchmark.Disabled('linux', 'win') @@ -67,6 +73,12 @@ page_set = page_sets.OortOnlineTBMPageSet + def GetExpectations(self): + class StoryExpectations(story.expectations.StoryExpectations): + def SetExpectations(self): + pass # http://oortonline.gl/#run not disabled. + return StoryExpectations() + def CreateTimelineBasedMeasurementOptions(self): categories = [ # Implicitly disable all categories.
diff --git a/tools/perf/benchmarks/rasterize_and_record_micro.py b/tools/perf/benchmarks/rasterize_and_record_micro.py index 92a6d35..c927b0c0 100644 --- a/tools/perf/benchmarks/rasterize_and_record_micro.py +++ b/tools/perf/benchmarks/rasterize_and_record_micro.py
@@ -57,6 +57,9 @@ def Name(cls): return 'rasterize_and_record_micro.top_25' + def GetExpectations(self): + return page_sets.Top25StoryExpectations() + @benchmark.Disabled('all') # http://crbug.com/531597 class RasterizeAndRecordMicroKeyMobileSites(_RasterizeAndRecordMicro): @@ -69,6 +72,9 @@ def Name(cls): return 'rasterize_and_record_micro.key_mobile_sites' + def GetExpectations(self): + return page_sets.KeyMobileSitesStoryExpectations() + @benchmark.Disabled('all') # http://crbug.com/610424 @benchmark.Owner(emails=['vmpstr@chromium.org']) @@ -84,6 +90,9 @@ def CreateStorySet(self, options): return page_sets.KeySilkCasesPageSet(run_no_page_interactions=True) + def GetExpectations(self): + return page_sets.KeySilkCasesStoryExpectations() + @benchmark.Disabled('all') # http://crbug.com/709561 class RasterizeAndRecordMicroPolymer(_RasterizeAndRecordMicro): @@ -98,6 +107,9 @@ def CreateStorySet(self, options): return page_sets.PolymerPageSet(run_no_page_interactions=True) + def GetExpectations(self): + return page_sets.PolymerRasterizeAndRecordStoryExpectations() + # New benchmark only enabled on Linux until we've observed behavior for a # reasonable period of time. @@ -112,4 +124,5 @@ def Name(cls): return 'rasterize_and_record_micro.partial_invalidation' - + def GetExpectations(self): + return page_sets.PartialInvalidationCasesStoryExpectations()
diff --git a/tools/perf/benchmarks/service_worker.py b/tools/perf/benchmarks/service_worker.py index 7d5acb9d..4662b7c 100644 --- a/tools/perf/benchmarks/service_worker.py +++ b/tools/perf/benchmarks/service_worker.py
@@ -174,6 +174,9 @@ def Name(cls): return 'service_worker.service_worker' + def GetExpectations(self): + return page_sets.ServiceWorkerStoryExpectations() + @benchmark.Disabled('android-webview') # http://crbug.com/653924 @benchmark.Owner(emails=['horo@chromium.org']) @@ -195,3 +198,6 @@ def ShouldDisable(cls, possible_browser): # http://crbug.com/597656 return (possible_browser.browser_type == 'reference' and possible_browser.platform.GetDeviceTypeName() == 'Nexus 5X') + + def GetExpectations(self): + return page_sets.ServiceWorkerMicroBenchmarksStoryExpectations()
diff --git a/tools/perf/benchmarks/speedometer.py b/tools/perf/benchmarks/speedometer.py index fbe431da..95a3950 100644 --- a/tools/perf/benchmarks/speedometer.py +++ b/tools/perf/benchmarks/speedometer.py
@@ -107,3 +107,9 @@ make_javascript_deterministic=False, name='http://browserbench.org/Speedometer/')) return ps + + def GetExpectations(self): + class StoryExpectations(story.expectations.StoryExpectations): + def SetExpectations(self): + pass # http://browserbench.org/Speedometer/ not disabled. + return StoryExpectations()
diff --git a/tools/perf/page_sets/indexeddb_endure_page.py b/tools/perf/page_sets/indexeddb_endure_page.py index 80feb6e..c14c761 100644 --- a/tools/perf/page_sets/indexeddb_endure_page.py +++ b/tools/perf/page_sets/indexeddb_endure_page.py
@@ -45,3 +45,8 @@ ] for test in tests: self.AddStory(IndexedDBEndurePage(test, self)) + + +class IndexedDBEndureStoryExpectations(story.expectations.StoryExpectations): + def SetExpectations(self): + pass # Nothing disabled.
diff --git a/tools/perf/page_sets/key_mobile_sites.py b/tools/perf/page_sets/key_mobile_sites.py index b3c0e81..7ce3b81 100644 --- a/tools/perf/page_sets/key_mobile_sites.py +++ b/tools/perf/page_sets/key_mobile_sites.py
@@ -93,9 +93,9 @@ action_on_load_complete=True)) # Why: #8 (Alexa global), picked an interesting page - # Forbidden (Rate Limit Exceeded) + # TODO(rnephew): Uncomment when this story is rerecorded. # self.AddStory(KeyMobileSitesPage( - # url='http://twitter.com/katyperry', page_set=self, name='Twitter')) + # url='http://twitter.com/katyperry', page_set=self, name='Twitter')) # Why: #37 (Alexa global) """ self.AddStory(KeyMobileSitesPage( @@ -104,13 +104,13 @@ name='Pinterest')) # Why: #1 sports. - # Fails often; crbug.com/249722' + # TODO(rnephew): Uncomment when this sotry is rerecorded. # self.AddStory(KeyMobileSitesPage( - # url='http://espn.go.com', page_set=self, name='ESPN')) + # url='http://espn.go.com', page_set=self, name='ESPN')) # Why: crbug.com/231413 - # Doesn't scroll; crbug.com/249736 - # self.AddStory(KeyMobileSitesPage( - # url='http://forecast.io', page_set=self)) + # TODO(rnephew): Uncomment when this story is rerecorded. + # self.AddStory( + # KeyMobileSitesPage(url='http://forecast.io', page_set=self)) # Why: crbug.com/169827 self.AddStory(KeyMobileSitesPage( url='http://slashdot.org/', page_set=self, tags=['fastpath'])) @@ -176,3 +176,13 @@ for url in urls_list: self.AddStory(KeyMobileSitesPage(url, self)) + +class KeyMobileSitesStoryExpectations(story.expectations.StoryExpectations): + def SetExpectations(self): + pass + # TODO(rnephew): Uncomment when these stories is rerecorded. + # self.DisableStory( + # 'http://forecast.io', [story.expectations.ALL], 'crbug.com/249736') + # self.DisableStory( + # 'Twitter', [story.expectations.ALL], 'Forbidden (Rate Limit Exceeded)') + # self.DisableStory('ESPN', [story.expectations.ALL], 'crbug.com/249722')
diff --git a/tools/perf/page_sets/key_silk_cases.py b/tools/perf/page_sets/key_silk_cases.py index ec2b4b4..96dd5daa 100644 --- a/tools/perf/page_sets/key_silk_cases.py +++ b/tools/perf/page_sets/key_silk_cases.py
@@ -748,12 +748,12 @@ self.AddStory(Page17(self, run_no_page_interactions)) self.AddStory(Page18(self, run_no_page_interactions)) # Missing frames during tap interaction; crbug.com/446332 - # self.AddStory(Page19(self, run_no_page_interactions)) + self.AddStory(Page19(self, run_no_page_interactions)) self.AddStory(Page20(self, run_no_page_interactions)) self.AddStory(GwsGoogleExpansion(self, run_no_page_interactions)) self.AddStory(GwsBoogieExpansion(self, run_no_page_interactions)) # Times out on Windows; crbug.com/338838 - # self.AddStory(Page22(self, run_no_page_interactions)) + self.AddStory(Page22(self, run_no_page_interactions)) self.AddStory(Page23(self, run_no_page_interactions)) self.AddStory(Page24(self, run_no_page_interactions)) self.AddStory(Page25(self, run_no_page_interactions)) @@ -762,7 +762,7 @@ self.AddStory(UpdateHistoryState(self, run_no_page_interactions)) self.AddStory(SilkFinance(self, run_no_page_interactions)) # Flaky interaction steps on Android; crbug.com/507865 - # self.AddStory(PolymerTopeka(self, run_no_page_interactions)) + self.AddStory(PolymerTopeka(self, run_no_page_interactions)) self.AddStory(Masonry(self, run_no_page_interactions)) for page in self: @@ -770,3 +770,13 @@ KeySilkCasesPage.RunPageInteractions), ( 'Pages in this page set must not override KeySilkCasesPage\' ' 'RunPageInteractions method.') + + +class KeySilkCasesStoryExpectations(story.expectations.StoryExpectations): + def SetExpectations(self): + self.DisableStory('https://polymer-topeka.appspot.com/', + [story.expectations.ALL], 'crbug.com/507865') + self.DisableStory('http://plus.google.com/app/basic/stream', + [story.expectations.ALL], 'crbug.com/338838') + self.DisableStory('inbox_app.html?slide_drawer', + [story.expectations.ALL], 'crbug.com/446332')
diff --git a/tools/perf/page_sets/oortonline.py b/tools/perf/page_sets/oortonline.py index 570630d6..4b59b8e 100644 --- a/tools/perf/page_sets/oortonline.py +++ b/tools/perf/page_sets/oortonline.py
@@ -68,6 +68,7 @@ with action_runner.CreateInteraction('End'): action_runner.tab.browser.DumpMemory() + class OortOnlineTBMPageSet(story.StorySet): """Oort Online WebGL benchmark for TBM. URL: http://oortonline.gl/#run
diff --git a/tools/perf/page_sets/partial_invalidation_cases.py b/tools/perf/page_sets/partial_invalidation_cases.py index 939df7be0..5bcbd7a 100644 --- a/tools/perf/page_sets/partial_invalidation_cases.py +++ b/tools/perf/page_sets/partial_invalidation_cases.py
@@ -35,3 +35,9 @@ for url in other_urls: self.AddStory(PartialInvalidationCasesPage(url, self)) + + +class PartialInvalidationCasesStoryExpectations( + story.expectations.StoryExpectations): + def SetExpectations(self): + pass # No tests disabled.
diff --git a/tools/perf/page_sets/polymer.py b/tools/perf/page_sets/polymer.py index fb465044..853efb99 100644 --- a/tools/perf/page_sets/polymer.py +++ b/tools/perf/page_sets/polymer.py
@@ -261,3 +261,8 @@ PolymerPage.RunPageInteractions), ( 'Pages in this page set must not override PolymerPage\' ' 'RunPageInteractions method.') + + +class PolymerRasterizeAndRecordStoryExpectations(story.expectations.StoryExpectations): + def SetExpectations(self): + pass # No tests disabled.
diff --git a/tools/perf/page_sets/service_worker.py b/tools/perf/page_sets/service_worker.py index 798bfc5..060d5ad2 100644 --- a/tools/perf/page_sets/service_worker.py +++ b/tools/perf/page_sets/service_worker.py
@@ -40,3 +40,8 @@ self.AddStory(page.Page( 'https://jakearchibald.github.io/svgomg/', self, name='svgomg_second_load', make_javascript_deterministic=False)) + + +class ServiceWorkerStoryExpectations(story.expectations.StoryExpectations): + def SetExpectations(self): + pass # Nothing disabled.
diff --git a/tools/perf/page_sets/service_worker_micro_benchmark.py b/tools/perf/page_sets/service_worker_micro_benchmark.py index 788defc..649c16b 100644 --- a/tools/perf/page_sets/service_worker_micro_benchmark.py +++ b/tools/perf/page_sets/service_worker_micro_benchmark.py
@@ -33,3 +33,8 @@ 'http://localhost:8091/index.html', self, make_javascript_deterministic=False, name='http://localhost:8091/index.html')) + +class ServiceWorkerMicroBenchmarksStoryExpectations( + story.expectations.StoryExpectations): + def SetExpectations(self): + pass # Nothing disabled.
diff --git a/tools/perf/page_sets/top_25_pages.py b/tools/perf/page_sets/top_25_pages.py index 2032d63..af23583 100644 --- a/tools/perf/page_sets/top_25_pages.py +++ b/tools/perf/page_sets/top_25_pages.py
@@ -41,8 +41,7 @@ # Why: #1 news worldwide (Alexa global) 'http://news.yahoo.com', # Why: #2 news worldwide - # crbug.com/528472 - #'http://www.cnn.com', + 'http://www.cnn.com', # Why: #1 world commerce website by visits; #3 commerce in the US by # time spent 'http://www.amazon.com', @@ -62,3 +61,8 @@ self.AddStory( page.Page(url, self, shared_page_state_class=shared_desktop_state, name=url)) + +class Top25StoryExpectations(story.expectations.StoryExpectations): + def SetExpectations(self): + self.DisableStory( + 'http://www.cnn.com', [story.expectations.ALL], 'crbug.com/528472')