Make the Chrome Web Store Icon Syncable

chrome/browser/extensions/extension_service.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/extensions/extension_service.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/basictypes.h"
@@ skipping 1555 matching lines @@
     return true;
 
   // Check the prefs.
   return extension_prefs_->IsIncognitoEnabled(extension_id);
 }
 
 void ExtensionService::SetIsIncognitoEnabled(
     const std::string& extension_id, bool enabled) {
   const Extension* extension = GetInstalledExtension(extension_id);
   if (extension && extension->location() == Extension::COMPONENT) {
-    // This shouldn't be called for component extensions.
-    NOTREACHED();
+    // This shouldn't be called for component extensions other than the

[Aaron Boodman, 2012/02/08 00:37:53]

Sorry, why does this happen - sync?

[csharp, 2012/02/08 15:46:16]

Yup.

+    // web store (which is considered an app, and may try to set this value).
+    DCHECK_EQ(extension_id, std::string(extension_misc::kWebStoreAppId));
+
+    // If we are here with the CWS, make sure the we aren't trying to
+    // change it.
+    DCHECK_EQ(enabled, IsIncognitoEnabled(extension_id));
+
     return;
   }
 
   // Broadcast unloaded and loaded events to update browser state. Only bother
   // if the value changed and the extension is actually enabled, since there is
   // no UI otherwise.
   bool old_enabled = extension_prefs_->IsIncognitoEnabled(extension_id);
   if (enabled == old_enabled)
     return;
 
@@ skipping 455 matching lines @@
     // disabled extension for other reasons other than that an update was
     // disabled, e.g. as in ExtensionManagementTest.InstallRequiresConfirm.
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_EXTENSION_UPDATE_DISABLED,
         content::Source<Profile>(profile_),
         content::Details<const Extension>(extension));
     SyncExtensionChangeIfNeeded(*extension);
     return;
   }
 
+  // Make sure that CWS has valid ordinals on initialization.
+  if (extension->id() == extension_misc::kWebStoreAppId) {

[akalin, 2012/02/07 22:54:13]

Why does this only apply to this particular app?  Why doesn't it apply to apps
in general?  Is it because CWS is the only app that can be installed before the
NTP shows up?  This should probably be explained in a comment.

[Aaron Boodman, 2012/02/08 00:37:53]

Can we just initialize all ordinals if they haven't already been done in
::AddExtension()?

Probably a better filter of what kinds of things to do this work for would be
extension->IsApp(). It doesn't really matter what the install location is. If
it's an app, it's going to show up in the launcher, and therefore should have an
ordinal (right?).

[csharp, 2012/02/08 15:46:16]

Good points. I'll initialize all uninitialized ordinals for apps here.

+    ExtensionSorting* extension_sorting = extension_prefs_->extension_sorting();
+
+    StringOrdinal page_ordinal =
+        extension_sorting->GetPageOrdinal(extension->id());
+    if (!page_ordinal.IsValid()) {
+      page_ordinal = extension_sorting->CreateFirstAppPageOrdinal();
+      extension_sorting->SetPageOrdinal(extension->id(), page_ordinal);
+    }
+
+    StringOrdinal app_launch_ordinal =
+        extension_sorting->GetAppLaunchOrdinal(extension->id());
+    if (!app_launch_ordinal.IsValid()) {
+      app_launch_ordinal =
+          extension_sorting->CreateFirstAppLaunchOrdinal(page_ordinal);
+      extension_sorting->SetAppLaunchOrdinal(extension->id(),
+                                             app_launch_ordinal);
+    }
+  }
+
   extensions_.Insert(scoped_extension);
   SyncExtensionChangeIfNeeded(*extension);
   NotifyExtensionLoaded(extension);
 }
 
 void ExtensionService::InitializePermissions(const Extension* extension) {
   // If the extension has used the optional permissions API, it will have a
   // custom set of active permissions defined in the extension prefs. Here,
   // we update the extension's active permissions based on the prefs.
   scoped_refptr<ExtensionPermissionSet> active_permissions =
@@ skipping 629 matching lines @@
   //
   // To coexist with certain unit tests that don't have an IO thread message
   // loop available at ExtensionService shutdown, we lazy-initialize this
   // object so that those cases neither create nor destroy a SocketController.
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (!socket_controller_) {
     socket_controller_ = new extensions::SocketController();
   }
   return socket_controller_;
 }