Prevent prefs_registrar unsubscribe from destroyed pref_service

chrome/browser/ui/webui/options/preferences_browsertest.cc

 // Copyright (c) 2012 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/ui/webui/options/preferences_browsertest.h"
 
 #include <stddef.h>
 
 #include <iostream>
 #include <memory>
@@ skipping 74 matching lines @@
 MATCHER_P(EqualsValue, expected, "") {
   return arg && arg->Equals(expected);
 }
 
 PreferencesBrowserTest::PreferencesBrowserTest() {
 }
 
 PreferencesBrowserTest::~PreferencesBrowserTest() {
 }
 
+PrefService* PreferencesBrowserTest::pref_service() {
+  DCHECK(pref_change_registrar_);
+  return pref_change_registrar_->prefs();
+}
+
 // Navigates to the settings page, causing the JavaScript pref handling code to
 // load and injects JavaScript testing code.
 void PreferencesBrowserTest::SetUpOnMainThread() {
   ui_test_utils::NavigateToURL(browser(),
                                GURL(chrome::kChromeUISettingsFrameURL));
   SetUpPrefs();
 }
 
+void PreferencesBrowserTest::TearDownOnMainThread() {
+  pref_change_registrar_.reset(nullptr);

[stevenjb, 2016/12/13 18:21:01]

nit: reset() ?

+}
+
 void PreferencesBrowserTest::SetUpPrefs() {
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_TRUE(web_contents);
   render_view_host_ = web_contents->GetRenderViewHost();
   ASSERT_TRUE(render_view_host_);
-  pref_service_ = browser()->profile()->GetPrefs();
-  pref_change_registrar_.Init(pref_service_);
+
+  DCHECK(!pref_change_registrar_);
+  pref_change_registrar_.reset(new PrefChangeRegistrar);
+  pref_change_registrar_->Init(browser()->profile()->GetPrefs());
+
   ASSERT_TRUE(content::ExecuteScript(render_view_host_,
       "function TestEnv() {"
       "  this.sentinelName_ = 'download.prompt_for_download';"
       "  this.prefs_ = [];"
       "  TestEnv.instance_ = this;"
       "}"
       ""
       "TestEnv.handleEvent = function(event) {"
       "  var env = TestEnv.instance_;"
       "  var name = event.type;"
@@ skipping 62 matching lines @@
       "  finishObservingAndReply: function() {"
       "    this.sentinelValue_ = !this.sentinelValue_;"
       "    Preferences.setBooleanPref(this.sentinelName_, this.sentinelValue_,"
       "                               true);"
       "  }"
       "};"));
 }
 
 // Forwards notifications received when pref values change in the backend.
 void PreferencesBrowserTest::OnPreferenceChanged(const std::string& pref_name) {
-  OnCommit(pref_service_->FindPreference(pref_name.c_str()));
+  OnCommit(pref_service()->FindPreference(pref_name.c_str()));
 }
 
 void PreferencesBrowserTest::SetUpInProcessBrowserTestFixture() {
   // Sets up a mock policy provider for user and device policies.
   EXPECT_CALL(policy_provider_, IsInitializationComplete(_))
       .WillRepeatedly(Return(true));
   policy::BrowserPolicyConnector::SetPolicyProviderForTesting(
       &policy_provider_);
 }
 
@@ skipping 11 matching lines @@
 
 void PreferencesBrowserTest::ClearUserPolicies() {
   policy::PolicyMap empty_policy_map;
   policy_provider_.UpdateChromePolicy(empty_policy_map);
 }
 
 void PreferencesBrowserTest::SetUserValues(
     const std::vector<std::string>& names,
     const std::vector<base::Value*>& values) {
   for (size_t i = 0; i < names.size(); ++i) {
-    pref_service_->Set(names[i].c_str(), *values[i]);
+    pref_service()->Set(names[i].c_str(), *values[i]);
   }
 }
 
 void PreferencesBrowserTest::VerifyKeyValue(const base::DictionaryValue& dict,
                                             const std::string& key,
                                             const base::Value& expected) {
   const base::Value* actual = NULL;
   EXPECT_TRUE(dict.Get(key, &actual)) << "Was checking key: " << key;
   if (actual)
     EXPECT_EQ(expected, *actual) << "Was checking key: " << key;
@@ skipping 52 matching lines @@
   const base::DictionaryValue* observed_dict;
   ASSERT_TRUE(observed_value_ptr.get());
   ASSERT_TRUE(observed_value_ptr->GetAsDictionary(&observed_dict));
   for (size_t i = 0; i < names.size(); ++i) {
     VerifyPref(observed_dict, names[i], values[i], controlledBy, disabled,
                uncommitted);
   }
 }
 
 void PreferencesBrowserTest::ExpectNoCommit(const std::string& name) {
-  pref_change_registrar_.Add(
+  pref_change_registrar_->Add(
       name.c_str(),
       base::Bind(&PreferencesBrowserTest::OnPreferenceChanged,
                  base::Unretained(this)));
   EXPECT_CALL(*this, OnCommit(Property(&PrefService::Preference::name, name)))
       .Times(0);
 }
 
 void PreferencesBrowserTest::ExpectSetCommit(const std::string& name,
                                              const base::Value* value) {
-  pref_change_registrar_.Add(
+  pref_change_registrar_->Add(
       name.c_str(),
       base::Bind(&PreferencesBrowserTest::OnPreferenceChanged,
                  base::Unretained(this)));
   EXPECT_CALL(*this, OnCommit(AllOf(
       Property(&PrefService::Preference::name, name),
       Property(&PrefService::Preference::IsUserControlled, true),
       Property(&PrefService::Preference::GetValue, EqualsValue(value)))));
 }
 
 void PreferencesBrowserTest::ExpectClearCommit(const std::string& name) {
-  pref_change_registrar_.Add(
+  pref_change_registrar_->Add(
       name.c_str(),
       base::Bind(&PreferencesBrowserTest::OnPreferenceChanged,
                  base::Unretained(this)));
   EXPECT_CALL(*this, OnCommit(AllOf(
       Property(&PrefService::Preference::name, name),
       Property(&PrefService::Preference::IsUserControlled, false))));
 }
 
 void PreferencesBrowserTest::VerifyAndClearExpectations() {
   Mock::VerifyAndClearExpectations(this);
-  pref_change_registrar_.RemoveAll();
+  pref_change_registrar_->RemoveAll();
 }
 
 void PreferencesBrowserTest::SetupJavaScriptTestEnvironment(
     const std::vector<std::string>& pref_names,
     std::string* observed_json) const {
   std::stringstream javascript;
   javascript << "var testEnv = new TestEnv();";
   for (std::vector<std::string>::const_iterator name = pref_names.begin();
        name != pref_names.end(); ++name) {
     javascript << "testEnv.addPref('" << name->c_str() << "');";
@@ skipping 133 matching lines @@
     base::ListValue* list = new base::ListValue;
     list->AppendString("http://www.example.com");
     list->AppendString("http://example.com");
     non_default_values_.push_back(list);
   }
 
   // Retrieve default values.
   for (std::vector<std::string>::const_iterator name = pref_names_.begin();
         name != pref_names_.end(); ++name) {
     default_values_.push_back(
-        pref_service_->GetDefaultPrefValue(name->c_str())->DeepCopy());
+        pref_service()->GetDefaultPrefValue(name->c_str())->DeepCopy());
   }
 }
 
 // Verifies that initializing the JavaScript Preferences class fires the correct
 // notifications in JavaScript.
 IN_PROC_BROWSER_TEST_F(PreferencesBrowserTest, FetchPrefs) {
   UseDefaultTestPrefs(true);
   std::string observed_json;
 
   // Verify notifications when default values are in effect.
@@ skipping 422 matching lines @@
     SetPref(name, type, &value, true, &observed_json);
   }
 
   void VerifyCurrentProxyServer(const std::string& expected_server,
                                 onc::ONCSource expected_source) {
     const chromeos::NetworkState* network = GetDefaultNetwork();
     ASSERT_TRUE(network);
     onc::ONCSource actual_source;
     std::unique_ptr<ProxyConfigDictionary> proxy_dict =
         chromeos::proxy_config::GetProxyConfigForNetwork(
-            g_browser_process->local_state(), pref_service_, *network,
+            g_browser_process->local_state(), pref_service(), *network,
             &actual_source);
     ASSERT_TRUE(proxy_dict);
     std::string actual_proxy_server;
     EXPECT_TRUE(proxy_dict->GetProxyServer(&actual_proxy_server));
     EXPECT_EQ(expected_server, actual_proxy_server);
     EXPECT_EQ(expected_source, actual_source);
   }
 };
 
 // Verifies that proxy settings are correctly pushed to JavaScript during
@@ skipping 156 matching lines @@
                base::FundamentalValue(3));
   SetProxyPref(chromeos::proxy_cros_settings_parser::kProxySocksPort,
                base::FundamentalValue(4));
 
   VerifyCurrentProxyServer(
       "http=a.com:1;https=4.3.2.1:2;ftp=c.com:3;socks=socks4://d.com:4",
       onc::ONC_SOURCE_NONE);
 }
 
 #endif