Enables storing shelf's auto_hide_behavior and alignment per display.

chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h"
 
 #include <vector>
 
 #include "ash/launcher/launcher_model.h"
 #include "ash/shell.h"
 #include "ash/wm/window_util.h"
 #include "base/command_line.h"
+#include "base/string_number_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/app_sync_ui_state.h"
 #include "chrome/browser/ui/ash/chrome_launcher_prefs.h"
@@ skipping 14 matching lines @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/common/url_pattern.h"
 #include "grit/theme_resources.h"
+#include "ui/aura/root_window.h"
 #include "ui/aura/window.h"
 
 using content::WebContents;
 using extensions::Extension;
 
 namespace {
 
 // Item controller for an app shortcut. Shortcuts track app and launcher ids,
 // but do not have any associated windows (opening a shortcut will replace the
 // item with the appropriate LauncherItemController type).
@@ skipping 54 matching lines @@
 
   // Stores the optional refocus url pattern for this item.
   const GURL& refocus_url() const { return refocus_url_; }
   void set_refocus_url(const GURL& refocus_url) { refocus_url_ = refocus_url; }
 
  private:
   GURL refocus_url_;
   DISALLOW_COPY_AND_ASSIGN(AppShortcutLauncherItemController);
 };
 
-// If the value of the pref at |local_path is not empty, it is returned
-// otherwise the value of the pref at |synced_path| is returned.
-std::string GetLocalOrRemotePref(PrefService* pref_service,
+std::string GetPrefKeyForRootWindow(aura::RootWindow* root_window) {
+  gfx::Display display = gfx::Screen::GetScreenFor(
+      root_window)->GetDisplayNearestWindow(root_window);
+  DCHECK(display.is_valid());
+
+  return base::Int64ToString(display.id());
+}
+
+void UpdatePerDisplayPref(PrefService* pref_service,
+                          aura::RootWindow* root_window,
+                          const char* pref_key,
+                          const std::string& value) {
+  std::string key = GetPrefKeyForRootWindow(root_window);
+  if (key.empty())
+    return;
+
+  DictionaryPrefUpdate update(pref_service, prefs::kShelfPreferences);
+  base::DictionaryValue* shelf_prefs = update.Get();
+  base::DictionaryValue* prefs = NULL;
+  if (!shelf_prefs->GetDictionary(key, &prefs)) {
+    prefs = new base::DictionaryValue();
+    shelf_prefs->Set(key, prefs);
+  }
+  prefs->SetStringWithoutPathExpansion(pref_key, value);
+}
+
+// Returns the pref in |pref_service| for |root_window|. The pref value

[bartfab (slow), 2012/12/14 21:34:02]

Nit: "Returns a pref value from |pref_service| for |root_window|." would be
clearer IMHO.

[Jun Mukai, 2012/12/14 22:44:56]

Done.

+// can vary among root windows and can be specified by the admin. Here is
+// the priority:

[bartfab (slow), 2012/12/14 21:34:02]

Nit: Turn the bullet points into faux sentences (capital letter at the start,
period at the end).

[Jun Mukai, 2012/12/14 22:44:56]

Done.

+//  * the value managed by the admin

[bartfab (slow), 2012/12/14 21:34:02]

1/ "Managed by the admin" is not the wording we usually use. "Managed by policy"
is the canonical expression.

2/ Please also state that this value is window-independent (i.e. the same value
for all windows).

[Jun Mukai, 2012/12/14 22:44:56]

Done.

+//  * a user-set value specific to the root window
+//  * the value recommended by the admin

[bartfab (slow), 2012/12/14 21:34:02]

1/ "Recommended by the admin" is not the wording we usually use. "Recommended by
policy" is the canonical expression.

2/ Please also state that this value is window-independent (i.e. the same value
for all windows).

[Jun Mukai, 2012/12/14 22:44:56]

Done.

+//  * otherwise, lookup the default value. |local_path| is preferred.

[bartfab (slow), 2012/12/14 21:34:02]

Nit: "look up" (two words).

[bartfab (slow), 2012/12/14 21:34:02]

"|local_path| is preferred" actually hides a lot of complexity behind three
simple words. I think this comment should be folded into the bullet points,
i.e.:

* A value managed by policy. This is a single value that applies to all windows.
It is stored at |local_path|.
* A user-set value. This value can be set per root window and is stored in a
per-window dict.
* A value recommended by policy. This is a single value that applies to all
windows. It is stored at |local_path|.
* A user-set value found at |local_path|. (maybe add an explanation here that
this comes from sync)
* A user-set or default value found at |path|.

[Jun Mukai, 2012/12/14 22:44:56]

Done.

[Jun Mukai, 2012/12/14 22:44:56]

This is slightly wrong actually.  User-set value should be prior to the
recommended one right? Following mostly but tweaking a bit.
I think the reason of |local_path| is preferred is too long and it's delegated
to the comment in pref_names.cc in this files, so I just noted here too.

+std::string GetPrefForRootWindow(PrefService* pref_service,
+                                 aura::RootWindow* root_window,
                                  const char* local_path,
-                                 const char* synced_path) {
+                                 const char* path) {
+  const PrefService::Preference* local_pref =
+      pref_service->FindPreference(local_path);
   const std::string value(pref_service->GetString(local_path));
-  return value.empty() ? pref_service->GetString(synced_path) : value;
+  if (local_pref->IsManaged())
+    return value;
+
+  std::string pref_key = GetPrefKeyForRootWindow(root_window);
+  if (!pref_key.empty()) {
+    const base::DictionaryValue* shelf_prefs = pref_service->GetDictionary(
+        prefs::kShelfPreferences);
+    const base::DictionaryValue* display_pref = NULL;
+    std::string per_display_value;
+    if (shelf_prefs->GetDictionary(pref_key, &display_pref) &&
+        display_pref->GetString(path, &per_display_value)) {
+      return per_display_value;
+    }
+  }
+
+  if (local_pref->IsRecommended())

[bartfab (slow), 2012/12/14 21:34:02]

This will work but I want to double-check the logic is the one you want:

If there is both a value at |local_path| or |path| and a recommended value,
which one should take precedence? The case where this happens is the first login
into a device. One pref value can be pulled in by sync, another pref value could
be recommended. Would it not be better to use the value pulled in from sync?
Arguably, it was a value the user chose. It should override the admin's
recommendation.

[Jun Mukai, 2012/12/14 22:44:56]

You make the point. Sure, the user pref from sync should override the
recommended value.  Fixed...

+    return value;
+
+  return value.empty() ? pref_service->GetString(path) : value;
 }
 
 // If prefs have synced and no user-set value exists at |local_path|, the value
 // from |synced_path| is copied to |local_path|.
 void MaybePropagatePrefToLocal(PrefService* pref_service,
                                const char* local_path,
                                const char* synced_path) {
   if (!pref_service->FindPreference(local_path)->HasUserSetting() &&
       pref_service->IsSyncing()) {
     // First time the user is using this machine, propagate from remote to
@@ skipping 418 matching lines @@
 
 bool ChromeLauncherControllerPerApp::CanPin() const {
   const PrefService::Preference* pref =
       profile_->GetPrefs()->FindPreference(prefs::kPinnedLauncherApps);
   return pref && pref->IsUserModifiable();
 }
 
 ash::ShelfAutoHideBehavior
     ChromeLauncherControllerPerApp::GetShelfAutoHideBehavior(
         aura::RootWindow* root_window) const {
-  // TODO(oshima): Support multiple launchers.
-
   // See comment in |kShelfAlignment| as to why we consider two prefs.
   const std::string behavior_value(
-      GetLocalOrRemotePref(profile_->GetPrefs(),
+      GetPrefForRootWindow(profile_->GetPrefs(),
+                           root_window,
                            prefs::kShelfAutoHideBehaviorLocal,
                            prefs::kShelfAutoHideBehavior));
 
   // Note: To maintain sync compatibility with old images of chrome/chromeos
   // the set of values that may be encountered includes the now-extinct
   // "Default" as well as "Never" and "Always", "Default" should now
   // be treated as "Never" (http://crbug.com/146773).
   if (behavior_value == ash::kShelfAutoHideBehaviorAlways)
     return ash::SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS;
   return ash::SHELF_AUTO_HIDE_BEHAVIOR_NEVER;
 }
 
 bool ChromeLauncherControllerPerApp::CanUserModifyShelfAutoHideBehavior(
     aura::RootWindow* root_window) const {
-  // TODO(oshima): Support multiple launchers.
   return profile_->GetPrefs()->
       FindPreference(prefs::kShelfAutoHideBehaviorLocal)->IsUserModifiable();
 }
 
 void ChromeLauncherControllerPerApp::ToggleShelfAutoHideBehavior(
     aura::RootWindow* root_window) {
   ash::ShelfAutoHideBehavior behavior = GetShelfAutoHideBehavior(root_window) ==
       ash::SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS ?
           ash::SHELF_AUTO_HIDE_BEHAVIOR_NEVER :
           ash::SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS;
@@ skipping 194 matching lines @@
       if (IsAppPinned(extension->id()))
         DoUnpinAppsWithID(extension->id());
       app_icon_loader_->ClearImage(extension->id());
       break;
     }
     default:
       NOTREACHED() << "Unexpected notification type=" << type;
   }
 }
 
-void ChromeLauncherControllerPerApp::OnShelfAlignmentChanged() {
+void ChromeLauncherControllerPerApp::OnShelfAlignmentChanged(
+    aura::RootWindow* root_window) {
   const char* pref_value = NULL;
-  // TODO(oshima): Support multiple displays.
-  switch (ash::Shell::GetInstance()->GetShelfAlignment(
-      ash::Shell::GetPrimaryRootWindow())) {
+  switch (ash::Shell::GetInstance()->GetShelfAlignment(root_window)) {
     case ash::SHELF_ALIGNMENT_BOTTOM:
       pref_value = ash::kShelfAlignmentBottom;
       break;
     case ash::SHELF_ALIGNMENT_LEFT:
       pref_value = ash::kShelfAlignmentLeft;
       break;
     case ash::SHELF_ALIGNMENT_RIGHT:
       pref_value = ash::kShelfAlignmentRight;
       break;
   }
-  // See comment in |kShelfAlignment| about why we have two prefs here.
-  profile_->GetPrefs()->SetString(prefs::kShelfAlignmentLocal, pref_value);
-  profile_->GetPrefs()->SetString(prefs::kShelfAlignment, pref_value);
+
+  UpdatePerDisplayPref(
+      profile_->GetPrefs(), root_window, prefs::kShelfAlignment, pref_value);
+
+  if (root_window == ash::Shell::GetPrimaryRootWindow()) {
+    // See comment in |kShelfAlignment| about why we have two prefs here.
+    profile_->GetPrefs()->SetString(prefs::kShelfAlignmentLocal, pref_value);
+    profile_->GetPrefs()->SetString(prefs::kShelfAlignment, pref_value);
+  }
 }
 
 void ChromeLauncherControllerPerApp::OnIsSyncingChanged() {
   MaybePropagatePrefToLocal(profile_->GetPrefs(),
                             prefs::kShelfAlignmentLocal,
                             prefs::kShelfAlignment);
   MaybePropagatePrefToLocal(profile_->GetPrefs(),
                             prefs::kShelfAutoHideBehaviorLocal,
                             prefs::kShelfAutoHideBehavior);
 }
@@ skipping 154 matching lines @@
   }
 
   // Append unprocessed items from the pref to the end of the model.
   for (; pref_app_id != pinned_apps.end(); ++pref_app_id)
     DoPinAppWithID(*pref_app_id);
 }
 
 void ChromeLauncherControllerPerApp::SetShelfAutoHideBehaviorPrefs(
     ash::ShelfAutoHideBehavior behavior,
     aura::RootWindow* root_window) {
-  // TODO(oshima): Support multiple launchers.
-  if (root_window != ash::Shell::GetPrimaryRootWindow())
-    return;
-
   const char* value = NULL;
   switch (behavior) {
     case ash::SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS:
       value = ash::kShelfAutoHideBehaviorAlways;
       break;
     case ash::SHELF_AUTO_HIDE_BEHAVIOR_NEVER:
       value = ash::kShelfAutoHideBehaviorNever;
       break;
   }
-  // See comment in |kShelfAlignment| about why we have two prefs here.
-  profile_->GetPrefs()->SetString(prefs::kShelfAutoHideBehaviorLocal, value);
-  profile_->GetPrefs()->SetString(prefs::kShelfAutoHideBehavior, value);
+
+  UpdatePerDisplayPref(
+      profile_->GetPrefs(), root_window, prefs::kShelfAutoHideBehavior, value);
+
+  if (root_window == ash::Shell::GetPrimaryRootWindow()) {
+    // See comment in |kShelfAlignment| about why we have two prefs here.
+    profile_->GetPrefs()->SetString(prefs::kShelfAutoHideBehaviorLocal, value);
+    profile_->GetPrefs()->SetString(prefs::kShelfAutoHideBehavior, value);
+  }
 }
 
 void ChromeLauncherControllerPerApp::SetShelfAutoHideBehaviorFromPrefs() {
-  // TODO(oshima): Support multiple displays.
-  aura::RootWindow* root_window = ash::Shell::GetPrimaryRootWindow();
-  ash::Shell::GetInstance()->SetShelfAutoHideBehavior(
-      GetShelfAutoHideBehavior(root_window), root_window);
+  ash::Shell::RootWindowList root_windows = ash::Shell::GetAllRootWindows();
+  for (ash::Shell::RootWindowList::const_iterator iter = root_windows.begin();
+       iter != root_windows.end(); ++iter) {
+    ash::Shell::GetInstance()->SetShelfAutoHideBehavior(
+        GetShelfAutoHideBehavior(*iter), *iter);
+  }
 }
 
 void ChromeLauncherControllerPerApp::SetShelfAlignmentFromPrefs() {
   if (!CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kShowLauncherAlignmentMenu))
     return;
 
-  // See comment in |kShelfAlignment| as to why we consider two prefs.
-  const std::string alignment_value(
-      GetLocalOrRemotePref(profile_->GetPrefs(),
-                           prefs::kShelfAlignmentLocal,
-                           prefs::kShelfAlignment));
-  ash::ShelfAlignment alignment = ash::SHELF_ALIGNMENT_BOTTOM;
-  if (alignment_value == ash::kShelfAlignmentLeft)
-    alignment = ash::SHELF_ALIGNMENT_LEFT;
-  else if (alignment_value == ash::kShelfAlignmentRight)
-    alignment = ash::SHELF_ALIGNMENT_RIGHT;
-  // TODO(oshima): Support multiple displays.
-  ash::Shell::GetInstance()->SetShelfAlignment(
-      alignment, ash::Shell::GetPrimaryRootWindow());
+  ash::Shell::RootWindowList root_windows = ash::Shell::GetAllRootWindows();
+  for (ash::Shell::RootWindowList::const_iterator iter = root_windows.begin();
+       iter != root_windows.end(); ++iter) {
+    // See comment in |kShelfAlignment| as to why we consider two prefs.
+    const std::string alignment_value(
+        GetPrefForRootWindow(profile_->GetPrefs(),
+                             *iter,
+                             prefs::kShelfAlignmentLocal,
+                             prefs::kShelfAlignment));
+    ash::ShelfAlignment alignment = ash::SHELF_ALIGNMENT_BOTTOM;
+    if (alignment_value == ash::kShelfAlignmentLeft)
+      alignment = ash::SHELF_ALIGNMENT_LEFT;
+    else if (alignment_value == ash::kShelfAlignmentRight)
+      alignment = ash::SHELF_ALIGNMENT_RIGHT;
+    ash::Shell::GetInstance()->SetShelfAlignment(alignment, *iter);
+  }
 }
 
 WebContents* ChromeLauncherControllerPerApp::GetLastActiveWebContents(
     const std::string& app_id) {
   AppIDToWebContentsListMap::const_iterator i =
       app_id_to_web_contents_list_.find(app_id);
   if (i == app_id_to_web_contents_list_.end())
     return NULL;
   DCHECK_GT(i->second.size(), 0u);
   return *i->second.begin();
@@ skipping 58 matching lines @@
 void ChromeLauncherControllerPerApp::SetAppIconLoaderForTest(
     AppIconLoader* loader) {
   app_icon_loader_.reset(loader);
 }
 
 const std::string&
 ChromeLauncherControllerPerApp::GetAppIdFromLauncherIdForTest(
     ash::LauncherID id) {
   return id_to_item_controller_map_[id]->app_id();
 }