[Settings] Fix notification settings crash due to multidevice policy.

This CL fixes crash in notifications settings page. This is reproduced on
devices where multidevice features are disabled by policy. In this case
MultiDeviceSetupClient is not instantiated, causing the look up for
android messages info in the notifications settings page to crash. This
was fixed by guarding the look up by a loadtime value that indicates
whether multidevice features are allowed policy or not.

Bug: 905908
Change-Id: Ic637768802e4e15b3643fe20dcc40d1bb3cea9da
Reviewed-on: https://chromium-review.googlesource.com/c/1345729
Commit-Queue: Azeem Arshad <azeemarshad@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611799}

chrome/browser/resources/settings/site_settings/site_list.js

@@ -277,6 +277,8 @@ Polymer({
     if (this.category === settings.ContentSettingsTypes.NOTIFICATIONS &&
         loadTimeData.valueExists('enableMultideviceSettings') &&
         loadTimeData.getBoolean('enableMultideviceSettings') &&
+        loadTimeData.valueExists('multideviceAllowedByPolicy') &&
+        loadTimeData.getBoolean('multideviceAllowedByPolicy') &&
         !this.androidSmsInfo_) {
       const multideviceSetupProxy =
           settings.MultiDeviceBrowserProxyImpl.getInstance();

chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

@@ -2707,10 +2707,6 @@ void AddMultideviceStrings(content::WebUIDataSource* html_source) {
   AddLocalizedStringsBulk(html_source, localized_strings,
                           arraysize(localized_strings));
 
-  html_source->AddBoolean(
-      "enableMultideviceSettings",
-      base::FeatureList::IsEnabled(
-          chromeos::features::kEnableUnifiedMultiDeviceSettings));
   html_source->AddString(
       "multideviceVerificationText",
       l10n_util::GetStringFUTF16(

chrome/browser/ui/webui/settings/md_settings_ui.cc

@@ -104,6 +104,7 @@
 #include "chromeos/account_manager/account_manager_factory.h"
 #include "chromeos/chromeos_features.h"
 #include "chromeos/chromeos_switches.h"
+#include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
 #include "components/arc/arc_util.h"
 #include "ui/base/ui_base_features.h"
 #else  // !defined(OS_CHROMEOS)
@@ -218,21 +219,6 @@ MdSettingsUI::MdSettingsUI(content::WebUI* web_ui)
   }
   AddSettingsPageUIHandler(
       std::make_unique<chromeos::settings::KeyboardHandler>());
-  if (!profile->IsGuestSession() &&
-      base::FeatureList::IsEnabled(
-          chromeos::features::kEnableUnifiedMultiDeviceSetup) &&
-      base::FeatureList::IsEnabled(
-          chromeos::features::kEnableUnifiedMultiDeviceSettings) &&
-      base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)) {
-    AddSettingsPageUIHandler(
-        std::make_unique<chromeos::settings::MultideviceHandler>(
-            profile->GetPrefs(),
-            chromeos::multidevice_setup::MultiDeviceSetupClientFactory::
-                GetForProfile(profile),
-            std::make_unique<
-                chromeos::multidevice_setup::AndroidSmsAppHelperDelegateImpl>(
-                profile)));
-  }
   AddSettingsPageUIHandler(
       std::make_unique<chromeos::settings::PointerHandler>());
   AddSettingsPageUIHandler(
@@ -296,6 +282,30 @@ MdSettingsUI::MdSettingsUI(content::WebUI* web_ui)
                           password_protection_available);
 
 #if defined(OS_CHROMEOS)
+  if (!profile->IsGuestSession() &&
+      base::FeatureList::IsEnabled(
+          chromeos::features::kEnableUnifiedMultiDeviceSetup) &&
+      base::FeatureList::IsEnabled(
+          chromeos::features::kEnableUnifiedMultiDeviceSettings) &&
+      base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)) {
+    AddSettingsPageUIHandler(
+        std::make_unique<chromeos::settings::MultideviceHandler>(
+            profile->GetPrefs(),
+            chromeos::multidevice_setup::MultiDeviceSetupClientFactory::
+                GetForProfile(profile),
+            std::make_unique<
+                chromeos::multidevice_setup::AndroidSmsAppHelperDelegateImpl>(
+                profile)));
+  }
+  html_source->AddBoolean(
+      "enableMultideviceSettings",
+      base::FeatureList::IsEnabled(
+          chromeos::features::kEnableUnifiedMultiDeviceSettings));
+  html_source->AddBoolean(
+      "multideviceAllowedByPolicy",
+      chromeos::multidevice_setup::AreAnyMultiDeviceFeaturesAllowed(
+          profile->GetPrefs()));
+
   AddSettingsPageUIHandler(base::WrapUnique(
       chromeos::settings::DateTimeHandler::Create(html_source)));
 

chrome/test/data/webui/settings/site_list_tests.js

@@ -272,6 +272,11 @@ suite('SiteList', function() {
    */
   let browserProxy = null;
 
+  /**
+   * Mock MultiDeviceBrowserProxy to use during test.
+   * @type {TestMultideviceBrowserProxy}
+   */
+  let multiDeviceBrowserProxy = null;
 
   suiteSetup(function() {
     CrSettingsPrefs.setInitialized();
@@ -292,8 +297,8 @@ suite('SiteList', function() {
     document.body.appendChild(testElement);
 
     if (cr.isChromeOS) {
-      settings.MultiDeviceBrowserProxyImpl.instance_ =
-          new multidevice.TestMultideviceBrowserProxy();
+      multiDeviceBrowserProxy = new multidevice.TestMultideviceBrowserProxy();
+      settings.MultiDeviceBrowserProxyImpl.instance_ = multiDeviceBrowserProxy;
     }
   });
 
@@ -305,7 +310,10 @@ suite('SiteList', function() {
 
     if (cr.isChromeOS) {
       // Reset multidevice enabled flag.
-      loadTimeData.overrideValues({enableMultideviceSettings: false});
+      loadTimeData.overrideValues({
+        enableMultideviceSettings: false,
+        multideviceAllowedByPolicy: false
+      });
     }
   });
 
@@ -395,8 +403,17 @@ suite('SiteList', function() {
       setUpCategory(
           settings.ContentSettingsTypes.NOTIFICATIONS,
           settings.ContentSetting.ALLOW, prefsAndroidSms);
-      const multiDeviceBrowserProxy =
-          settings.MultiDeviceBrowserProxyImpl.getInstance();
+      assertEquals(
+          0, multiDeviceBrowserProxy.getCallCount('getAndroidSmsInfo'));
+
+      loadTimeData.overrideValues({multideviceAllowedByPolicy: true});
+      setUpCategory(
+          settings.ContentSettingsTypes.NOTIFICATIONS,
+          settings.ContentSetting.ALLOW, prefsAndroidSms);
+      // Assert 2 calls since the observer observes 2 properties.
+      assertEquals(
+          2, multiDeviceBrowserProxy.getCallCount('getAndroidSmsInfo'));
+
       return multiDeviceBrowserProxy.whenCalled('getAndroidSmsInfo')
           .then(() => browserProxy.whenCalled('getExceptionList'))
           .then((contentType) => {