Make the Chrome Web Store Icon Syncable

chrome/browser/extensions/extension_service.cc

Index: chrome/browser/extensions/extension_service.cc
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 84397982cc4e10874b4d7d1a24dfd29d71d0adef..a7664dd809b6e6a9903e811f83a31707882359d5 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -1573,8 +1573,14 @@ 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;
   }
 
@@ -2050,6 +2056,27 @@ void ExtensionService::AddExtension(const Extension* 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);