MD Settings: don't allow supervised users to switch to GTK+ theme

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

 // 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.
 
 /** @fileoverview Runs polymer appearance font settings elements. */
 
 cr.define('settings_appearance', function() {
   /**
    * A test version of AppearanceBrowserProxy.
    *
    * @constructor
    * @implements {settings.AppearanceBrowserProxy}
    * @extends {settings.TestBrowserProxy}
    */
   var TestAppearanceBrowserProxy = function() {
     settings.TestBrowserProxy.call(this, [
       'getThemeInfo',
+      'isSupervised',
       'openWallpaperManager',
       'useDefaultTheme',
       'useSystemTheme',
     ]);
   };
 
   TestAppearanceBrowserProxy.prototype = {
     __proto__: settings.TestBrowserProxy.prototype,
 
+    /** @private */
+    isSupervised_: false,
+
     /** @override */
     getThemeInfo: function(themeId) {
       this.methodCalled('getThemeInfo', themeId);
       return Promise.resolve({name: 'Sports car red'});
     },
 
     /** @override */
+    isSupervised: function() {
+      this.methodCalled('isSupervised');
+      return this.isSupervised_;
+    },
+
+    /** @override */
     openWallpaperManager: function() {
       this.methodCalled('openWallpaperManager');
     },
 
     /** @override */
     useDefaultTheme: function() {
       this.methodCalled('useDefaultTheme');
     },
 
     /** @override */
     useSystemTheme: function() {
       this.methodCalled('useSystemTheme');
     },
+
+    /** @param {boolean} Whether the user is supervised */
+    setIsSupervised: function(isSupervised) {
+      this.isSupervised_ = isSupervised;
+    },
   };
 
   /**
    * A test version of FontsBrowserProxy.
    *
    * @constructor
    * @implements {settings.FontsBrowserProxy}
    * @extends {settings.TestBrowserProxy}
    */
   var TestFontsBrowserProxy = function() {
@@ skipping 41 matching lines @@
         appearanceBrowserProxy = new TestAppearanceBrowserProxy();
         settings.AppearanceBrowserProxyImpl.instance_ = appearanceBrowserProxy;
 
         PolymerTest.clearBody();
 
         appearancePage = document.createElement('settings-appearance-page');
         appearancePage.set('prefs', {
           extensions: {
             theme: {
               id: {
-                value: 'asdf',
+                value: '',
               },
               use_system: {
                 value: false,
               },
             },
           },
         });
         document.body.appendChild(appearancePage);
         Polymer.dom.flush();
       });
 
       teardown(function() { appearancePage.remove(); });
 
       if (cr.isChromeOS) {
         test('wallpaperManager', function() {
           var button = appearancePage.$.wallpaperButton;
           assertTrue(!!button);
           MockInteractions.tap(button);
           return appearanceBrowserProxy.whenCalled('openWallpaperManager');
         });
       } else {
         test('noWallpaperManager', function() {
           // The wallpaper button should not be present.
           var button = appearancePage.$.wallpaperButton;
           assertFalse(!!button);
         });
       }
 
       test('useDefaultTheme', function() {
+        assertFalse(!!appearancePage.get('prefs.extensions.theme.id.value'));
+        assertFalse(appearancePage.useSystemTheme_);

[dpapad, 2016/10/18 19:18:00]

Is there any way to avoid referring to private state of the appearance page
here? Perhaps dropping this assertion, keeping only assertions that refer to the
DOM, and at line 159, instead of setting the private var directly, somehow
trigger the useSystemThemePrefChanged_ properly (maybe by actually changing the
pref)?

[Dan Beam, 2016/10/18 19:27:44]

the pref only exists on linux

[dpapad, 2016/10/18 19:37:27]

Ok. Can we split the the default and system themes test then? And wrap the
"system" theme test in an 
if (cr.isLinux && !cr.isChromeOS) {...}, similar to the next test? See also
question at line 181. 

[Dan Beam, 2016/10/18 21:50:25]

Done.

+        // No custom nor system theme in use; the "RESET TO DEFAULT" or "USE
+        // CLASSIC" should be hidden.
+        assertFalse(!!appearancePage.$$('#useDefault'));
+
+        appearancePage.useSystemTheme_ = true;
+        Polymer.dom.flush();
+        // The system theme only currently exists on Linux, but the code is
+        // cross-platform. If the system theme is in use, "USE CLASSIC" should
+        // show.
+        assertTrue(!!appearancePage.$$('#useDefault'));
+
+        appearancePage.useSystemTheme_ = false;
+        appearancePage.set('prefs.extensions.theme.id.value', 'fake theme id');
+        Polymer.dom.flush();
+
+        // With a custom theme installed, "USE CLASSIC"/"RESET" should show.
         var button = appearancePage.$$('#useDefault');
         assertTrue(!!button);
+
         MockInteractions.tap(button);
         return appearanceBrowserProxy.whenCalled('useDefaultTheme');
       });
 
       if (cr.isLinux && !cr.isChromeOS) {
         test('useSystemTheme', function() {
+          assertFalse(!!appearancePage.get('prefs.extensions.theme.id.value'));
+          appearancePage.useSystemTheme_ = true;

[dpapad, 2016/10/18 19:37:27]

This tests only runs on Linux and therefore the pref should exist. Can we avoid
referring to private member vars of the advanced page here and use the pref to
trigger the codepaths to be tested?

[Dan Beam, 2016/10/18 21:50:25]

Done.

+          Polymer.dom.flush();
+          // The "USE GTK+" button shouldn't be showing if it's already in use.
+          assertFalse(!!appearancePage.$$('#useSystem'));
+
+          appearanceBrowserProxy.setIsSupervised(true);
+          appearancePage.useSystemTheme_ = false;
+          Polymer.dom.flush();
+          // Supervised users have their own theme and can't use GTK+ theme.
+          assertFalse(!!appearancePage.$$('#useDefault'));
+          assertFalse(!!appearancePage.$$('#useSystem'));
+          // If there's no "USE" buttons, the container should be hidden.
+          assertTrue(appearancePage.$$('.secondary-action').hidden);
+
+          appearanceBrowserProxy.setIsSupervised(false);
+          appearancePage.set('prefs.extensions.theme.id.value', 'fake theme');
+          Polymer.dom.flush();
+          // If there's "USE" buttons again, the container should be visible.
+          assertTrue(!!appearancePage.$$('#useDefault'));
+          assertFalse(appearancePage.$$('.secondary-action').hidden);
+
           var button = appearancePage.$$('#useSystem');
           assertTrue(!!button);
+
           MockInteractions.tap(button);
           return appearanceBrowserProxy.whenCalled('useSystemTheme');
         });
       }
     });
   }
 
   function registerAppearanceFontSettingsBrowserTest() {
     var fontsPage = null;
 
@@ skipping 28 matching lines @@
     });
   }
 
   return {
     registerTests: function() {
       registerAppearanceFontSettingsBrowserTest();
       registerAppearanceSettingsBrowserTest();
     },
   };
 });