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

chrome/browser/extensions/extension_service.cc

 // Copyright (c) 2011 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/bind.h"
@@ skipping 1693 matching lines @@
 void ExtensionService::CheckForUpdatesSoon() {
   if (updater()) {
     updater()->CheckSoon();
   } else {
     LOG(WARNING) << "CheckForUpdatesSoon() called with auto-update turned off";
   }
 }
 
 namespace {
   bool IsSyncableNone(const Extension& extension) { return false; }
-} // namespace
+}  // namespace
 
 ExtensionService::SyncBundle::SyncBundle()
   : filter(IsSyncableNone),
     sync_processor(NULL) {
 }
 
 ExtensionService::SyncBundle::~SyncBundle() {
 }
 
 bool ExtensionService::SyncBundle::HasExtensionId(const std::string& id) const {
@@ skipping 210 matching lines @@
   // Handle uninstalls first.
   if (extension_sync_data.uninstalled()) {
     std::string error;
     if (!UninstallExtensionHelper(this, id)) {
       LOG(WARNING) << "Could not uninstall extension " << id
                    << " for sync";
     }
     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());
     if (result < 0) {
       // Extension is outdated.
       bundle.pending_sync_data[extension_sync_data.id()] = extension_sync_data;
       CheckForUpdatesSoon();
     }
   } else {
     // TODO(akalin): Replace silent update with a list of enabled
     // permissions.
     const bool kInstallSilently = true;
     if (!pending_extension_manager()->AddFromSync(
             id,
             extension_sync_data.update_url(),
             bundle.filter,
             kInstallSilently)) {
       LOG(WARNING) << "Could not add pending extension for " << id;
       // This means that the extension is already pending installation, with a
       // non-INTERNAL location.  Add to pending_sync_data, even though it will
       // never be removed (we'll never install a syncable version of the
       // extension), so that GetAllSyncData() continues to send it.
     }
     // Track pending extensions so that we can return them in GetAllSyncData().
     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(
     const std::string& extension_id) const {
   // If this is an existing component extension we always allow it to
   // work in incognito mode.
   const Extension* extension = GetInstalledExtension(extension_id);
   if (extension && extension->location() == Extension::COMPONENT)
     return true;
 
@@ skipping 12 matching lines @@
 
   // 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;
 
   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) {
   // We allow the extension to see events and data from another profile iff it
   // uses "spanning" behavior and it has incognito access. "split" mode
   // extensions only see events for a matching profile.
   CHECK(extension);
   return IsIncognitoEnabled(extension->id()) &&
       !extension->incognito_split_mode();
 }
@@ skipping 981 matching lines @@
 
 ExtensionService::NaClModuleInfoList::iterator
     ExtensionService::FindNaClModule(const GURL& url) {
   for (NaClModuleInfoList::iterator iter = nacl_module_list_.begin();
        iter != nacl_module_list_.end(); ++iter) {
     if (iter->url == url)
       return iter;
   }
   return nacl_module_list_.end();
 }