Fixed null pointer crash in ExtensionSettingsHandler.

chrome/browser/ui/webui/extensions/extension_settings_handler.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/ui/webui/extensions/extension_settings_handler.h"
 
 #include "apps/app_load_service.h"
 #include "apps/app_restore_service.h"
 #include "apps/app_window.h"
 #include "apps/app_window_registry.h"
@@ skipping 825 matching lines @@
 
 void ExtensionSettingsHandler::HandleEnableMessage(
     const base::ListValue* args) {
   CHECK_EQ(2U, args->GetSize());
   std::string extension_id, enable_str;
   CHECK(args->GetString(0, &extension_id));
   CHECK(args->GetString(1, &enable_str));
 
   const Extension* extension =
       extension_service_->GetInstalledExtension(extension_id);
-  if (!extension ||
-      !management_policy_->UserMayModifySettings(extension, NULL)) {
+  if (!extension)
+    return;
+
+  if (!management_policy_->UserMayModifySettings(extension, NULL)) {
     LOG(ERROR) << "Attempt to enable an extension that is non-usermanagable was"
                << "made. Extension id: " << extension->id();
     return;
   }

[Finnur, 2014/03/07 12:55:53]

Wow... this gave me a serious sense of deja-vu! And rightly so, because I fixed
this exact bug in another class, but didn't realize it existed here also.
https://codereview.chromium.org/23496030/

However, I would prefer if you fixed it in the same manner as I did back then...
(see CL).

[vasilii, 2014/03/07 13:02:00]

That means the log message is wrong. What should I do with other places in this
file which simply return?

[Finnur, 2014/03/07 13:44:03]

If you are sure that we should not output this LOG message when |extension| is
NULL (which I think is likely, but wasn't sure how the ManagementPolicy stuff
worked), then your fix is better and the other file needs to change as well.

But if not, I'd like to evaluate "these other places" you refer to.

Also, the wording of the LOG message is bothering me a bit. Can you change it to
say: 
"An attempt was made to enable an extension that is non-usermanagable. Extension
id: " << extension->id();

[vasilii, 2014/03/07 15:16:55]

We definitely shouldn't output this message. The bug I'm fixing is the UI bug. A
user clicks "delete extension" and shortly something else. Thus, Chrome tries to
perform some action on the uninstalled extension.
The question is if we want to LOG() something else in this scenario?

[Finnur, 2014/03/07 16:27:16]

Nah, probably not. We return early for NULL extensions in a lot of places in
this file and we don't LOG that.

But I'd make the same changes in the other file. And with that, LGTM.

 
   if (enable_str == "true") {
     ExtensionPrefs* prefs = ExtensionPrefs::Get(extension_service_->profile());
     if (prefs->DidExtensionEscalatePermissions(extension_id)) {
       ShowExtensionDisabledDialog(
           extension_service_, web_ui()->GetWebContents(), extension);
     } else if ((prefs->GetDisableReasons(extension_id) &
                    Extension::DISABLE_UNSUPPORTED_REQUIREMENT) &&
                !requirements_checker_.get()) {
       // Recheck the requirements.
@@ skipping 385 matching lines @@
     extension_service_->EnableExtension(extension_id);
   } else {
     ExtensionErrorReporter::GetInstance()->ReportError(
         base::UTF8ToUTF16(JoinString(requirement_errors, ' ')),
         true /* be noisy */);
   }
   requirements_checker_.reset();
 }
 
 }  // namespace extensions