mash: Update shelf pin prefs in ShelfModelObserver overrides.

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

 // Copyright 2013 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.
 
 #ifndef CHROME_BROWSER_UI_ASH_LAUNCHER_CHROME_LAUNCHER_CONTROLLER_H_
 #define CHROME_BROWSER_UI_ASH_LAUNCHER_CHROME_LAUNCHER_CONTROLLER_H_
 
 #include <memory>
 #include <string>
 #include <vector>
 
 #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"
 #include "chrome/browser/ui/ash/launcher/launcher_item_controller.h"
 #include "chrome/browser/ui/ash/launcher/settings_window_observer.h"
 #include "mojo/public/cpp/bindings/associated_binding.h"
 
 class AccountId;
 class ArcAppDeferredLauncherController;
 class BrowserShortcutLauncherItemController;
@@ skipping 200 matching lines @@
   // Get the launch ID for a given shelf ID.
   virtual const std::string& GetLaunchIDForShelfID(ash::ShelfID id) = 0;
 
   AppIconLoader* GetAppIconLoaderForApp(const std::string& app_id);
 
   // Sets the shelf auto-hide and/or alignment behavior from prefs.
   void SetShelfAutoHideBehaviorFromPrefs();
   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);
   void SetAppIconLoadersForTest(
       std::vector<std::unique_ptr<AppIconLoader>>& loaders);
 
   void SetProfileForTest(Profile* profile);
 
  protected:
   ChromeLauncherController();
@@ skipping 36 matching lines @@
 
   // Ash's mojom::ShelfController used to change shelf state.
   ash::mojom::ShelfControllerPtr shelf_controller_;
 
   // The binding this instance uses to implment mojom::ShelfObserver
   mojo::AssociatedBinding<ash::mojom::ShelfObserver> observer_binding_;
 
   // 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_;
 
   // An observer that manages the shelf title and icon for settings windows.
   SettingsWindowObserver settings_window_observer_;
 
   // Used to load the images for app items.
   std::vector<std::unique_ptr<AppIconLoader>> app_icon_loaders_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeLauncherController);
 };
 
 #endif  // CHROME_BROWSER_UI_ASH_LAUNCHER_CHROME_LAUNCHER_CONTROLLER_H_