Fix a crash in PageActionImageView when extensions are reloaded due to incognito settings changing.

chrome/browser/extensions/extension_service.cc

Index: chrome/browser/extensions/extension_service.cc
===================================================================
--- chrome/browser/extensions/extension_service.cc	(revision 107550)
+++ chrome/browser/extensions/extension_service.cc	(working copy)
@@ -1711,7 +1711,7 @@
 
 namespace {
   bool IsSyncableNone(const Extension& extension) { return false; }
-} // namespace
+}  // namespace
 
 ExtensionService::SyncBundle::SyncBundle()
   : filter(IsSyncableNone),
@@ -1942,14 +1942,6 @@
     return;
   }
 
-  // Set user settings.
-  if (extension_sync_data.enabled()) {
-    EnableExtension(id);
-  } else {
-    DisableExtension(id);
-  }
-  SetIsIncognitoEnabled(id, extension_sync_data.incognito_enabled());

[Finnur, 2011/10/27 15:23:32]

I had to move this down to the bottom of the function because using the
extension pointer is not safe after calling SetIsIncognitoEnabled (it reloads
and invalidates the extension pointer).

Can either bolms or akalin verify it is OK to move this code?

[akalin, 2011/10/31 20:26:49]

I'd prefer leaving this block as is, but caching any info you need before
calling SetIsIncognitoEnabled (and adding a comment why).

It looks like it's only needed to see if the extension exists, and if so, to
compare its version.

-
   if (extension) {
     // If the extension is already installed, check if it's outdated.
     int result = extension->version()->CompareTo(extension_sync_data.version());
@@ -1977,6 +1969,14 @@
     bundle.pending_sync_data[extension_sync_data.id()] = extension_sync_data;
     CheckForUpdatesSoon();
   }
+
+  // Set user settings.
+  if (extension_sync_data.enabled()) {
+    EnableExtension(id);
+  } else {
+    DisableExtension(id);
+  }
+  SetIsIncognitoEnabled(id, extension_sync_data.incognito_enabled());
 }
 
 bool ExtensionService::IsIncognitoEnabled(
@@ -2009,17 +2009,10 @@
 
   extension_prefs_->SetIsIncognitoEnabled(extension_id, enabled);
 
-  // If the extension is enabled (and not terminated), unload and
-  // reload it to update UI.
-  const Extension* enabled_extension = GetExtensionById(extension_id, false);
-  if (enabled_extension) {
-    NotifyExtensionUnloaded(
-        enabled_extension, extension_misc::UNLOAD_REASON_DISABLE);
-    NotifyExtensionLoaded(enabled_extension);
-  }
-
-  if (extension)
-    SyncExtensionChangeIfNeeded(*extension);

[Finnur, 2011/10/27 15:23:32]

These last two lines are not needed because ReloadExtension does this.

+  bool extension_is_enabled = std::find(extensions_.begin(), extensions_.end(),
+                                        extension) != extensions_.end();
+  if (extension_is_enabled)
+    ReloadExtension(extension->id());
 }
 
 bool ExtensionService::CanCrossIncognito(const Extension* extension) {