mash: Update shelf pin prefs in ShelfModelObserver overrides.

chrome/browser/ui/ash/launcher/chrome_launcher_controller.h

Index: chrome/browser/ui/ash/launcher/chrome_launcher_controller.h
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h
index 3741cfdf7d5dc0d576de6df9a910aecfa9f6477c..0fdb264fcde31405533ad281688cbeccb915c322 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h
@@ -12,6 +12,7 @@
 #include "ash/public/cpp/app_launch_id.h"
 #include "ash/public/cpp/shelf_item.h"
 #include "ash/public/interfaces/shelf.mojom.h"
+#include "base/auto_reset.h"
 #include "chrome/browser/ui/app_icon_loader.h"
 #include "chrome/browser/ui/app_icon_loader_delegate.h"
 #include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
@@ -232,6 +233,11 @@ class ChromeLauncherController : public ash::mojom::ShelfObserver,
   void SetShelfAlignmentFromPrefs();
   void SetShelfBehaviorsFromPrefs();
 
+  bool should_sync_pin_changes() const { return should_sync_pin_changes_; }
+
+  // Temporarily prevent pinned shelf item changes from updating the sync model.
+  std::unique_ptr<base::AutoReset<bool>> DoNotSyncPinChanges();

[James Cook, 2017/03/29 23:23:56]

hrm. I like the idea of returning a RAII-style "lock" but it's confusing that
you're returning an AutoReset. This might be clearer if the AutoReset was
typedef'd to something like a SyncPinBlocker or BlockSyncPinsLock. Or you could
use a trivial ScopedDoNotSyncPins class.

[msw, 2017/03/30 00:33:29]

Elliot suggests adding move semantics to AutoReset, but that would require a
change to base (I could do that in a follow-up). For now, I added |using
PinSyncingDisabler = std::unique_ptr<base::AutoReset<bool>>;| to
ChromeLauncherController, in tribute to the beautiful snowflake that is
SigninScreenPolicyProvider::GetScopedSigninScreenPolicyProviderDisablerForTesting()
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/extensions/signi...

I also considered these type names: MoveableAutoResetBool (too verbose?),
CLC::PinSyncLock (not really a typical lock acquired for a shared resource,
etc.?), None of these solutions are particularly appealing to me, but I'll take
whichever approach you like best.

[James Cook, 2017/03/30 02:05:28]

I think "using" is fine for now.

optional name idea: ScopedPinSyncDisabler?

[msw, 2017/03/30 02:22:11]

Done.

+
   // Sets LauncherControllerHelper or AppIconLoader for test, taking ownership.
   void SetLauncherControllerHelperForTest(
       std::unique_ptr<LauncherControllerHelper> helper);
@@ -288,6 +294,9 @@ class ChromeLauncherController : public ash::mojom::ShelfObserver,
   // True when setting a shelf pref in response to an observer notification.
   bool updating_shelf_pref_from_observer_ = false;
 
+  // When true, changes to pinned shelf items should update the sync model.
+  bool should_sync_pin_changes_ = true;
+
   // Used to get app info for tabs.
   std::unique_ptr<LauncherControllerHelper> launcher_controller_helper_;