chromeos: Fix crash when clicking "Clear browsing data" in settings

I recently added the settings-sync-account-control element to the
Chrome OS build:
https://chromium-review.googlesource.com/c/chromium/src/+/1978844

The "Clear browsing data" WebUI dialog contains that element. Its
Polymer attached() method calls PeopleHandler::GetStoredAccountList
which crashes in guest mode because no IdentityManager exists.

This didn't crash before because the Polymer element wasn't
included in the Chrome OS build, so the DOM element did nothing.

Make the C++ handler work in guest mode by returning an empty list.
This fixes the crash.

Also use a dom-if to remove the settings-sync-account-control from
the dialog footer. The Chromium Polymer style guide recommends dom-if
when a hidden section contains custom elements.

Bug: 1040476
Test: added automated tests
Change-Id: Id2f703d0c37784c6525dc9127281530d87231421
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1999238
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#731575}

chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html

@@ -256,28 +256,30 @@
             $i18n{clearData}
         </cr-button>
       </div>
-      <div slot="footer"
-          hidden="[[!shouldShowFooter_(syncStatus.signedIn)]]">
-        <settings-sync-account-control sync-status="[[syncStatus]]"
-            prefs="{{prefs}}" hide-buttons>
-        </settings-sync-account-control>
-        <div class="divider"></div>
-        <div id="footer-description" on-click="onSyncDescriptionLinkClicked_">
-          <span id="sync-info" hidden="[[syncStatus.hasError]]">
-            $i18nRaw{clearBrowsingDataWithSync}
-          </span>
-          <span id="sync-paused-info" hidden="[[!isSyncPaused_]]">
-            $i18nRaw{clearBrowsingDataWithSyncPaused}
-          </span>
-          <span id="sync-passphrase-error-info"
-              hidden="[[!hasPassphraseError_]]">
-            $i18nRaw{clearBrowsingDataWithSyncPassphraseError}
-          </span>
-          <span id="sync-other-error-info" hidden="[[!hasOtherSyncError_]]">
-            $i18nRaw{clearBrowsingDataWithSyncError}
-          </span>
+      <template is="dom-if" if="[[shouldShowFooter_(syncStatus.signedIn)]]"
+          restamp>
+        <div slot="footer">
+          <settings-sync-account-control sync-status="[[syncStatus]]"
+              prefs="{{prefs}}" hide-buttons>
+          </settings-sync-account-control>
+          <div class="divider"></div>
+          <div id="footer-description" on-click="onSyncDescriptionLinkClicked_">
+            <span id="sync-info" hidden="[[syncStatus.hasError]]">
+              $i18nRaw{clearBrowsingDataWithSync}
+            </span>
+            <span id="sync-paused-info" hidden="[[!isSyncPaused_]]">
+              $i18nRaw{clearBrowsingDataWithSyncPaused}
+            </span>
+            <span id="sync-passphrase-error-info"
+                hidden="[[!hasPassphraseError_]]">
+              $i18nRaw{clearBrowsingDataWithSyncPassphraseError}
+            </span>
+            <span id="sync-other-error-info" hidden="[[!hasOtherSyncError_]]">
+              $i18nRaw{clearBrowsingDataWithSyncError}
+            </span>
+          </div>
         </div>
-      </div>
+      </template>
     </cr-dialog>
 
     <cr-dialog id="installedAppsDialog" close-text="$i18n{close}"

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

@@ -541,6 +541,9 @@ base::Value PeopleHandler::GetStoredAccountsList() {
     return accounts;
   }
 #endif
+  // Guest mode does not have a primary account (or an IdentityManager).
+  if (profile_->IsGuestSession())
+    return base::ListValue();
   // If dice is disabled or unsupported, show only the primary account.
   auto* identity_manager = IdentityManagerFactory::GetForProfile(profile_);
   base::Optional<AccountInfo> primary_account_info =

chrome/browser/ui/webui/settings/people_handler.h

@@ -114,6 +114,7 @@ class PeopleHandler : public SettingsPageUIHandler,
                            DashboardClearWhileSettingsOpen_ConfirmLater);
   FRIEND_TEST_ALL_PREFIXES(PeopleHandlerDiceUnifiedConsentTest,
                            StoredAccountsList);
+  FRIEND_TEST_ALL_PREFIXES(PeopleHandlerGuestModeTest, GetStoredAccountsList);
 
   // SettingsPageUIHandler implementation.
   void RegisterMessages() override;

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

@@ -1224,7 +1224,20 @@ TEST(PeopleHandlerDiceUnifiedConsentTest, StoredAccountsList) {
   EXPECT_EQ("a@gmail.com", accounts_list[0].FindKey("email")->GetString());
   EXPECT_EQ("b@gmail.com", accounts_list[1].FindKey("email")->GetString());
 }
-
 #endif  // BUILDFLAG(ENABLE_DICE_SUPPORT)
 
+#if defined(OS_CHROMEOS)
+// Regression test for crash in guest mode. https://crbug.com/1040476
+TEST(PeopleHandlerGuestModeTest, GetStoredAccountsList) {
+  content::BrowserTaskEnvironment task_environment;
+  TestingProfile::Builder builder;
+  builder.SetGuestSession();
+  std::unique_ptr<Profile> profile = builder.Build();
+
+  PeopleHandler handler(profile.get());
+  base::Value accounts = handler.GetStoredAccountsList();
+  EXPECT_TRUE(accounts.GetList().empty());
+}
+#endif  // defined(OS_CHROMEOS)
+
 }  // namespace settings

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

@@ -314,8 +314,7 @@ cr.define('settings_privacy_page', function() {
           hasError: false,
         });
         Polymer.dom.flush();
-        const footer = element.$$('#clearBrowsingDataDialog [slot=footer]');
-        assertTrue(footer.hidden);
+        assertFalse(!!element.$$('#clearBrowsingDataDialog [slot=footer]'));
 
         // Syncing: the footer is shown, with the normal sync info.
         cr.webUIListenerCallback('sync-status-changed', {
@@ -323,7 +322,7 @@ cr.define('settings_privacy_page', function() {
           hasError: false,
         });
         Polymer.dom.flush();
-        assertFalse(footer.hidden);
+        assertTrue(!!element.$$('#clearBrowsingDataDialog [slot=footer]'));
         assertVisible(element.$$('#sync-info'), true);
         assertVisible(element.$$('#sync-paused-info'), false);
         assertVisible(element.$$('#sync-passphrase-error-info'), false);
@@ -372,8 +371,7 @@ cr.define('settings_privacy_page', function() {
           hasError: false,
         });
         Polymer.dom.flush();
-        assertFalse(
-            element.$$('#clearBrowsingDataDialog [slot=footer]').hidden);
+        assertTrue(!!element.$$('#clearBrowsingDataDialog [slot=footer]'));
         const syncInfo = element.$$('#sync-info');
         assertVisible(syncInfo, true);
         const signoutLink = syncInfo.querySelector('a[href]');
@@ -390,8 +388,7 @@ cr.define('settings_privacy_page', function() {
           statusAction: settings.StatusAction.REAUTHENTICATE,
         });
         Polymer.dom.flush();
-        assertFalse(
-            element.$$('#clearBrowsingDataDialog [slot=footer]').hidden);
+        assertTrue(!!element.$$('#clearBrowsingDataDialog [slot=footer]'));
         const syncInfo = element.$$('#sync-paused-info');
         assertVisible(syncInfo, true);
         const signinLink = syncInfo.querySelector('a[href]');
@@ -408,8 +405,7 @@ cr.define('settings_privacy_page', function() {
           statusAction: settings.StatusAction.ENTER_PASSPHRASE,
         });
         Polymer.dom.flush();
-        assertFalse(
-            element.$$('#clearBrowsingDataDialog [slot=footer]').hidden);
+        assertTrue(!!element.$$('#clearBrowsingDataDialog [slot=footer]'));
         const syncInfo = element.$$('#sync-passphrase-error-info');
         assertVisible(syncInfo, true);
         const passphraseLink = syncInfo.querySelector('a[href]');
@@ -617,8 +613,7 @@ cr.define('settings_privacy_page', function() {
             hasError: false,
           });
           Polymer.dom.flush();
-          assertTrue(
-              element.$$('#clearBrowsingDataDialog [slot=footer]').hidden);
+          assertFalse(!!element.$$('#clearBrowsingDataDialog [slot=footer]'));
 
           // Syncing.
           cr.webUIListenerCallback('sync-status-changed', {
@@ -626,8 +621,7 @@ cr.define('settings_privacy_page', function() {
             hasError: false,
           });
           Polymer.dom.flush();
-          assertTrue(
-              element.$$('#clearBrowsingDataDialog [slot=footer]').hidden);
+          assertFalse(!!element.$$('#clearBrowsingDataDialog [slot=footer]'));
 
           // Sync passphrase error.
           cr.webUIListenerCallback('sync-status-changed', {
@@ -636,8 +630,7 @@ cr.define('settings_privacy_page', function() {
             statusAction: settings.StatusAction.ENTER_PASSPHRASE,
           });
           Polymer.dom.flush();
-          assertTrue(
-              element.$$('#clearBrowsingDataDialog [slot=footer]').hidden);
+          assertFalse(!!element.$$('#clearBrowsingDataDialog [slot=footer]'));
 
           // Other sync error.
           cr.webUIListenerCallback('sync-status-changed', {
@@ -646,8 +639,7 @@ cr.define('settings_privacy_page', function() {
             statusAction: settings.StatusAction.NO_ACTION,
           });
           Polymer.dom.flush();
-          assertTrue(
-              element.$$('#clearBrowsingDataDialog [slot=footer]').hidden);
+          assertFalse(!!element.$$('#clearBrowsingDataDialog [slot=footer]'));
         });
       }
     });