[CrOS SplitSettings] Move wallpaper references out of AppearanceHandler.

Remove Wallpaper references out of AppearanceHandler into a new WallpaperHandler. Let browser settings use WallpaperHandler and AppearanceHandler for now as SplitSetting is not default yet, and os settings use WallpaperHandler only. Bug: 971813 Change-Id: I66e8b574e37f78e3cb86cdf0ad7f0cab046252c6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1726624 Commit-Queue: Regan Hsu <hsuregan@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> Cr-Commit-Position: refs/heads/master@{#685040}
chromium · Aug 8, 2019 · 3462d51 · 3462d51
1 parent 335539e
commit 3462d51
Show file tree

Hide file tree

Showing 25 changed files with 271 additions and 223 deletions.
diff --git a/chrome/browser/resources/settings/appearance_page/BUILD.gn b/chrome/browser/resources/settings/appearance_page/BUILD.gn
@@ -12,6 +12,10 @@ js_type_check("closure_compile") {
     ":fonts_browser_proxy",
     ":home_url_input",
   ]
+
+  if (is_chromeos) {
+    deps += [ ":wallpaper_browser_proxy" ]
+  }
 }
 
 js_library("appearance_fonts_page") {
@@ -42,6 +46,7 @@ js_library("appearance_browser_proxy") {
 js_library("appearance_page") {
   deps = [
     ":appearance_browser_proxy",
+    ":wallpaper_browser_proxy",
     "..:page_visibility",
     "..:route",
     "../controls:settings_dropdown_menu",
@@ -75,3 +80,10 @@ js_library("home_url_input") {
   ]
   externs_list = [ "$externs_path/settings_private.js" ]
 }
+
+js_library("wallpaper_browser_proxy") {
+  deps = [
+    "//ui/webui/resources/js:cr",
+  ]
+  externs_list = [ "$externs_path/chrome_send.js" ]
+}
diff --git a/chrome/browser/resources/settings/appearance_page/appearance_browser_proxy.js b/chrome/browser/resources/settings/appearance_page/appearance_browser_proxy.js
@@ -17,22 +17,6 @@ cr.define('settings', function() {
     /** @return {boolean} Whether the current profile is supervised. */
     isSupervised() {}
 
-    /**
-     * @return {!Promise<boolean>} Whether the wallpaper setting row should be
-     *     visible.
-     */
-    isWallpaperSettingVisible() {}
-
-    /**
-     * @return {!Promise<boolean>} Whether the wallpaper is policy controlled.
-     */
-    isWallpaperPolicyControlled() {}
-
-    // <if expr="chromeos">
-    openWallpaperManager() {}
-
-    // </if>
-
     useDefaultTheme() {}
 
     // <if expr="is_linux and not chromeos">
@@ -70,24 +54,6 @@ cr.define('settings', function() {
       return loadTimeData.getBoolean('isSupervised');
     }
 
-    // <if expr="chromeos">
-    /** @override */
-    isWallpaperSettingVisible() {
-      return cr.sendWithPromise('isWallpaperSettingVisible');
-    }
-
-    /** @override */
-    isWallpaperPolicyControlled() {
-      return cr.sendWithPromise('isWallpaperPolicyControlled');
-    }
-
-    /** @override */
-    openWallpaperManager() {
-      chrome.send('openWallpaperManager');
-    }
-
-    // </if>
-
     /** @override */
     useDefaultTheme() {
       chrome.send('useDefaultTheme');

diff --git a/chrome/browser/resources/settings/appearance_page/appearance_page.html b/chrome/browser/resources/settings/appearance_page/appearance_page.html
@@ -18,6 +18,10 @@
 <link rel="import" href="appearance_fonts_page.html">
 <link rel="import" href="home_url_input.html">
 
+<if expr="chromeos">
+<link rel="import" href="../appearance_page/wallpaper_browser_proxy.html">
+</if>
+
 <dom-module id="settings-appearance-page">
   <template>
     <style include="settings-shared md-select iron-flex">

diff --git a/chrome/browser/resources/settings/appearance_page/appearance_page.js b/chrome/browser/resources/settings/appearance_page/appearance_page.js
@@ -119,7 +119,12 @@ Polymer({
   },
 
   /** @private {?settings.AppearanceBrowserProxy} */
-  browserProxy_: null,
+  appearanceBrowserProxy_: null,
+
+  // <if expr="chromeos">
+  /** @private {?settings.WallpaperBrowserProxy} */
+  wallpaperBrowserProxy_: null,
+  // </if>
 
   observers: [
     'defaultFontSizeChanged_(prefs.webkit.webprefs.default_font_size.value)',
@@ -133,24 +138,29 @@ Polymer({
 
   /** @override */
   created: function() {
-    this.browserProxy_ = settings.AppearanceBrowserProxyImpl.getInstance();
+    this.appearanceBrowserProxy_ =
+        settings.AppearanceBrowserProxyImpl.getInstance();
+    // <if expr="chromeos">
+    this.wallpaperBrowserProxy_ =
+        settings.WallpaperBrowserProxyImpl.getInstance();
+    // </if>
   },
 
   /** @override */
   ready: function() {
     this.$.defaultFontSize.menuOptions = this.fontSizeOptions_;
     // TODO(dschuyler): Look into adding a listener for the
     // default zoom percent.
-    this.browserProxy_.getDefaultZoom().then(zoom => {
+    this.appearanceBrowserProxy_.getDefaultZoom().then(zoom => {
       this.defaultZoom_ = zoom;
     });
     // <if expr="chromeos">
-    this.browserProxy_.isWallpaperSettingVisible().then(
+    this.wallpaperBrowserProxy_.isWallpaperSettingVisible().then(
         isWallpaperSettingVisible => {
           assert(this.pageVisibility);
           this.pageVisibility.setWallpaper = isWallpaperSettingVisible;
         });
-    this.browserProxy_.isWallpaperPolicyControlled().then(
+    this.wallpaperBrowserProxy_.isWallpaperPolicyControlled().then(
         isPolicyControlled => {
           this.isWallpaperPolicyControlled_ = isPolicyControlled;
         });
@@ -219,13 +229,13 @@ Polymer({
    * @private
    */
   openWallpaperManager_: function() {
-    this.browserProxy_.openWallpaperManager();
+    this.wallpaperBrowserProxy_.openWallpaperManager();
   },
   // </if>
 
   /** @private */
   onUseDefaultTap_: function() {
-    this.browserProxy_.useDefaultTheme();
+    this.appearanceBrowserProxy_.useDefaultTheme();
   },
 
   // <if expr="is_linux and not chromeos">
@@ -254,7 +264,8 @@ Polymer({
    * @private
    */
   showUseSystem_: function(themeId, useSystemTheme) {
-    return (!!themeId || !useSystemTheme) && !this.browserProxy_.isSupervised();
+    return (!!themeId || !useSystemTheme) &&
+        !this.appearanceBrowserProxy_.isSupervised();
   },
 
   /**
@@ -271,7 +282,7 @@ Polymer({
 
   /** @private */
   onUseSystemTap_: function() {
-    this.browserProxy_.useSystemTheme();
+    this.appearanceBrowserProxy_.useSystemTheme();
   },
   // </if>
 
@@ -288,7 +299,7 @@ Polymer({
     if (themeId.length > 0 && themeId != AUTOGENERATED_THEME_ID) {
       assert(!useSystemTheme);
 
-      this.browserProxy_.getThemeInfo(themeId).then(info => {
+      this.appearanceBrowserProxy_.getThemeInfo(themeId).then(info => {
         this.themeSublabel_ = info.name;
       });
 

diff --git a/chrome/browser/resources/settings/appearance_page/wallpaper_browser_proxy.html b/chrome/browser/resources/settings/appearance_page/wallpaper_browser_proxy.html
@@ -0,0 +1,2 @@
+<link rel="href" src="chrome://resources/html/cr.html">
+<script src="wallpaper_browser_proxy.js"></script>
diff --git a/...ion_page/personalization_browser_proxy.js → ...ppearance_page/wallpaper_browser_proxy.js b/...ion_page/personalization_browser_proxy.js → ...ppearance_page/wallpaper_browser_proxy.js
@@ -4,7 +4,7 @@
 
 cr.define('settings', function() {
   /** @interface */
-  class PersonalizationBrowserProxy {
+  class WallpaperBrowserProxy {
     /**
      * @return {!Promise<boolean>} Whether the wallpaper setting row should be
      *     visible.
@@ -20,9 +20,9 @@ cr.define('settings', function() {
   }
 
   /**
-   * @implements {settings.PersonalizationBrowserProxy}
+   * @implements {settings.WallpaperBrowserProxy}
    */
-  class PersonalizationBrowserProxyImpl {
+  class WallpaperBrowserProxyImpl {
     /** @override */
     isWallpaperSettingVisible() {
       return cr.sendWithPromise('isWallpaperSettingVisible');
@@ -39,10 +39,10 @@ cr.define('settings', function() {
     }
   }
 
-  cr.addSingletonGetter(PersonalizationBrowserProxyImpl);
+  cr.addSingletonGetter(WallpaperBrowserProxyImpl);
 
   return {
-    PersonalizationBrowserProxy: PersonalizationBrowserProxy,
-    PersonalizationBrowserProxyImpl: PersonalizationBrowserProxyImpl,
+    WallpaperBrowserProxy: WallpaperBrowserProxy,
+    WallpaperBrowserProxyImpl: WallpaperBrowserProxyImpl,
   };
 });
diff --git a/chrome/browser/resources/settings/chromeos/personalization_page/BUILD.gn b/chrome/browser/resources/settings/chromeos/personalization_page/BUILD.gn
@@ -6,21 +6,13 @@ import("//third_party/closure_compiler/compile_js.gni")
 
 js_type_check("closure_compile") {
   deps = [
-    ":personalization_browser_proxy",
     ":personalization_page",
   ]
 }
 
-js_library("personalization_browser_proxy") {
-  deps = [
-    "//ui/webui/resources/js:cr",
-  ]
-  externs_list = [ "$externs_path/chrome_send.js" ]
-}
-
 js_library("personalization_page") {
   deps = [
-    ":personalization_browser_proxy",
+    "../../appearance_page:wallpaper_browser_proxy",
     "../../settings_page:settings_animated_pages",
     "//ui/webui/resources/js:cr",
   ]

diff --git a/...owser/resources/settings/chromeos/personalization_page/personalization_browser_proxy.html b/...owser/resources/settings/chromeos/personalization_page/personalization_browser_proxy.html
diff --git a/chrome/browser/resources/settings/chromeos/personalization_page/personalization_page.html b/chrome/browser/resources/settings/chromeos/personalization_page/personalization_page.html
@@ -1,7 +1,7 @@
 <link rel="import" href="chrome://resources/html/polymer.html">
 
 <link rel="import" href="chrome://resources/cr_elements/cr_link_row/cr_link_row.html">
-<link rel="import" href="personalization_browser_proxy.html">
+<link rel="import" href="../../appearance_page/wallpaper_browser_proxy.html">
 <link rel="import" href="../../people_page/change_picture.html">
 <link rel="import" href="../../route.html">
 <link rel="import" href="../../settings_page/settings_animated_pages.html">

diff --git a/chrome/browser/resources/settings/chromeos/personalization_page/personalization_page.js b/chrome/browser/resources/settings/chromeos/personalization_page/personalization_page.js
@@ -32,12 +32,12 @@ Polymer({
     },
   },
 
-  /** @private {?settings.PersonalizationBrowserProxy} */
+  /** @private {?settings.WallpaperBrowserProxy} */
   browserProxy_: null,
 
   /** @override */
   created: function() {
-    this.browserProxy_ = settings.PersonalizationBrowserProxyImpl.getInstance();
+    this.browserProxy_ = settings.WallpaperBrowserProxyImpl.getInstance();
   },
 
   /** @override */

diff --git a/chrome/browser/resources/settings/os_settings_resources.grd b/chrome/browser/resources/settings/os_settings_resources.grd
@@ -125,12 +125,6 @@
                  file="settings_page/settings_animated_pages.js"
                  type="chrome_html"
                  preprocess="true" />
-      <structure name="IDR_OS_SETTINGS_PERSONALIZATION_BROWSER_PROXY_HTML"
-                 file="chromeos/personalization_page/personalization_browser_proxy.html"
-                 type="chrome_html" />
-      <structure name="IDR_OS_SETTINGS_PERSONALIZATION_BROWSER_PROXY_JS"
-                 file="chromeos/personalization_page/personalization_browser_proxy.js"
-                 type="chrome_html" />
       <structure name="IDR_OS_SETTINGS_PERSONALIZATION_PAGE_HTML"
                  file="chromeos/personalization_page/personalization_page.html"
                  type="chrome_html"
@@ -1351,6 +1345,12 @@
                  type="chrome_html"
                  preprocess="true"
                  allowexternalscript="true" />
+      <structure name="IDR_OS_SETTINGS_WALLPAPER_BROWSER_PROXY_HTML"
+                 file="appearance_page/wallpaper_browser_proxy.html"
+                 type="chrome_html" />
+      <structure name="IDR_OS_SETTINGS_WALLPAPER_BROWSER_PROXY_JS"
+                 file="appearance_page/wallpaper_browser_proxy.js"
+                 type="chrome_html" />
     </structures>
   </release>
 </grit>
diff --git a/chrome/browser/resources/settings/settings_resources.grd b/chrome/browser/resources/settings/settings_resources.grd
@@ -151,6 +151,14 @@
                  file="appearance_page/appearance_browser_proxy.js"
                  type="chrome_html"
                  preprocess="true" />
+      <if expr="chromeos">
+        <structure name="IDR_SETTINGS_WALLPAPER_BROWSER_PROXY_HTML"
+                   file="appearance_page/wallpaper_browser_proxy.html"
+                   type="chrome_html" />
+        <structure name="IDR_SETTINGS_WALLPAPER_BROWSER_PROXY_JS"
+                   file="appearance_page/wallpaper_browser_proxy.js"
+                   type="chrome_html" />
+      </if>
       <structure name="IDR_SETTINGS_APPEARANCE_FONTS_PAGE_HTML"
                  file="appearance_page/appearance_fonts_page.html"
                  type="chrome_html"

diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
@@ -1854,6 +1854,8 @@ jumbo_split_static_library("ui") {
       "webui/settings/chromeos/parental_controls_handler.h",
       "webui/settings/chromeos/plugin_vm_handler.cc",
       "webui/settings/chromeos/plugin_vm_handler.h",
+      "webui/settings/chromeos/wallpaper_handler.cc",
+      "webui/settings/chromeos/wallpaper_handler.h",
       "webui/settings/tts_handler.cc",
       "webui/settings/tts_handler.h",
       "webui/signin/inline_login_handler_chromeos.cc",

diff --git a/chrome/browser/ui/webui/settings/appearance_handler.cc b/chrome/browser/ui/webui/settings/appearance_handler.cc
@@ -12,10 +12,6 @@
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "content/public/browser/web_ui.h"
 
-#if defined(OS_CHROMEOS)
-#include "chrome/browser/ui/ash/wallpaper_controller_client.h"
-#endif
-
 namespace settings {
 
 AppearanceHandler::AppearanceHandler(content::WebUI* webui)
@@ -37,22 +33,6 @@ void AppearanceHandler::RegisterMessages() {
       base::BindRepeating(&AppearanceHandler::HandleUseSystemTheme,
                           base::Unretained(this)));
 #endif
-#if defined(OS_CHROMEOS)
-  web_ui()->RegisterMessageCallback(
-      "openWallpaperManager",
-      base::BindRepeating(&AppearanceHandler::HandleOpenWallpaperManager,
-                          base::Unretained(this)));
-
-  web_ui()->RegisterMessageCallback(
-      "isWallpaperSettingVisible",
-      base::BindRepeating(&AppearanceHandler::IsWallpaperSettingVisible,
-                          base::Unretained(this)));
-
-  web_ui()->RegisterMessageCallback(
-      "isWallpaperPolicyControlled",
-      base::BindRepeating(&AppearanceHandler::IsWallpaperPolicyControlled,
-                          base::Unretained(this)));
-#endif
 }
 
 void AppearanceHandler::HandleUseDefaultTheme(const base::ListValue* args) {
@@ -68,31 +48,4 @@ void AppearanceHandler::HandleUseSystemTheme(const base::ListValue* args) {
 }
 #endif
 
-#if defined(OS_CHROMEOS)
-void AppearanceHandler::IsWallpaperSettingVisible(const base::ListValue* args) {
-  CHECK_EQ(args->GetSize(), 1U);
-  bool result = WallpaperControllerClient::Get()->ShouldShowWallpaperSetting();
-  ResolveCallback(args->GetList()[0], result);
-}
-
-void AppearanceHandler::IsWallpaperPolicyControlled(
-    const base::ListValue* args) {
-  CHECK_EQ(args->GetSize(), 1U);
-  bool result = WallpaperControllerClient::Get()
-                    ->IsActiveUserWallpaperControlledByPolicy();
-  ResolveCallback(args->GetList()[0], result);
-}
-
-void AppearanceHandler::HandleOpenWallpaperManager(
-    const base::ListValue* args) {
-  WallpaperControllerClient::Get()->OpenWallpaperPickerIfAllowed();
-}
-
-void AppearanceHandler::ResolveCallback(const base::Value& callback_id,
-                                        bool result) {
-  AllowJavascript();
-  ResolveJavascriptCallback(callback_id, base::Value(result));
-}
-#endif
-
 }  // namespace settings