SplitSettings: People icon in browser settings should not be clickable

Split settings is moving the "Change account image" functionality to
OS settings. UX wants the item in browser settings not to be clickable
anymore. See bug.

Screenshot of non-clickable row:
http://screen/1nrWpZxXMBb

Bug: 990528
Test: added to browser_tests CrSettingsPeoplePageTest.All
Change-Id: I83a9c04f5c8a792a88375bef25cddb090ec4ec3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1745267Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Auto-Submit: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685402}

chrome/browser/resources/settings/people_page/people_page.html

@@ -151,12 +151,13 @@
 </if>
           <div id="picture-subpage-trigger" class="settings-box first two-line">
             <template is="dom-if" if="[[syncStatus]]">
-              <div id="profile-icon" on-click="onProfileTap_" actionable
+              <div id="profile-icon" on-click="onProfileTap_"
+                  actionable$="[[allowChangePicture_]]"
                   style="background-image: [[getIconImageSet_(
                       profileIconUrl_)]]">
               </div>
               <div class="middle two-line no-min-width" on-click="onProfileTap_"
-                  actionable>
+                  actionable$="[[allowChangePicture_]]">
                 <div class="flex text-elide settings-box-text">
                   <span id="profile-name">[[profileName_]]</span>
                   <div class="secondary" hidden="[[!syncStatus.signedIn]]">
@@ -170,6 +171,8 @@
 </if>
 <if expr="chromeos">
                 <cr-icon-button class="subpage-arrow"
+                    id="profile-subpage-arrow"
+                    hidden="[[!allowChangePicture_]]"
                     aria-label="$i18n{changePictureTitle}"
                     aria-describedby="profile-name"></cr-icon-button>
 </if>

chrome/browser/resources/settings/people_page/people_page.js

@@ -90,6 +90,20 @@ Polymer({
      */
     profileIconUrl_: String,
 
+    /**
+     * Whether clicking on the profile row should take the user to the
+     * "change picture" sub-page.
+     * @private
+     */
+    allowChangePicture_: {
+      type: Boolean,
+      value: function() {
+        // On Chrome OS, only allow when SplitSettings is disabled.
+        return cr.isChromeOS ? loadTimeData.getBoolean('showOSSettings') : true;
+      },
+      readOnly: true,
+    },
+
     /**
      * The current profile name.
      * @private
@@ -342,7 +356,11 @@ Polymer({
   /** @private */
   onProfileTap_: function() {
     // <if expr="chromeos">
-    settings.navigateTo(settings.routes.CHANGE_PICTURE);
+    if (this.allowChangePicture_) {
+      // Testing allowChangePicture_ is simpler than conditionally removing
+      // on-click handlers in the HTML.
+      settings.navigateTo(settings.routes.CHANGE_PICTURE);
+    }
     // </if>
     // <if expr="not chromeos">
     settings.navigateTo(settings.routes.MANAGE_PROFILE);

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

@@ -585,4 +585,61 @@ cr.define('settings_people_page', function() {
       assertEquals(settings.getCurrentRoute(), settings.routes.SYNC);
     });
   });
+
+  if (cr.isChromeOS) {
+    suite('Chrome OS with SplitSettings', function() {
+      /** @type {SettingsPeoplePageElement} */
+      let peoplePage = null;
+      /** @type {settings.SyncBrowserProxy} */
+      let browserProxy = null;
+      /** @type {settings.ProfileInfoBrowserProxy} */
+      let profileInfoBrowserProxy = null;
+
+      suiteSetup(function() {
+        loadTimeData.overrideValues({
+          // Simulate SplitSettings (OS settings in their own surface).
+          showOSSettings: false,
+        });
+      });
+
+      setup(async function() {
+        browserProxy = new TestSyncBrowserProxy();
+        settings.SyncBrowserProxyImpl.instance_ = browserProxy;
+
+        profileInfoBrowserProxy = new TestProfileInfoBrowserProxy();
+        settings.ProfileInfoBrowserProxyImpl.instance_ =
+            profileInfoBrowserProxy;
+
+        PolymerTest.clearBody();
+        peoplePage = document.createElement('settings-people-page');
+        peoplePage.pageVisibility = settings.pageVisibility;
+        document.body.appendChild(peoplePage);
+
+        Polymer.dom.flush();
+        await browserProxy.whenCalled('getSyncStatus');
+      });
+
+      teardown(function() {
+        peoplePage.remove();
+      });
+
+      test('clicking profile row does not open change picture page', () => {
+        // Simulate a signed-in user.
+        sync_test_util.simulateSyncStatus({
+          signedIn: true,
+        });
+
+        // Profile row items aren't actionable.
+        const profileIcon = assert(peoplePage.$$('#profile-icon'));
+        assertFalse(profileIcon.hasAttribute('actionable'));
+        const subpageArrow = assert(peoplePage.$$('#profile-subpage-arrow'));
+        assertFalse(subpageArrow.hasAttribute('actionable'));
+
+        // Clicking on profile icon doesn't navigate to a new route.
+        const oldRoute = settings.getCurrentRoute();
+        profileIcon.click();
+        assertEquals(oldRoute, settings.getCurrentRoute());
+      });
+    });
+  }
 });