Fix a crash on clicking "Load unpacked extension" multiple times

chrome/browser/ui/webui/extensions/extension_loader_handler.cc

 // Copyright 2014 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_loader_handler.h"
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
@@ skipping 81 matching lines @@
   // it doesn't try and call back.
   if (load_extension_dialog_.get())
     load_extension_dialog_->ListenerDestroyed();
 }
 
 void ExtensionLoaderHandler::FileHelper::ChooseFile() {
   static const int kFileTypeIndex = 0;  // No file type information to index.
   static const ui::SelectFileDialog::Type kSelectType =
       ui::SelectFileDialog::SELECT_FOLDER;
 
-  if (!load_extension_dialog_.get()) {
-    load_extension_dialog_ = ui::SelectFileDialog::Create(
-        this,
-        new ChromeSelectFilePolicy(
-            loader_handler_->web_ui()->GetWebContents()));
-  }
+  load_extension_dialog_ = ui::SelectFileDialog::Create(

[Devlin, 2014/12/03 17:47:55]

hmm... I have a slight concern in that ui::SelectFileDialog is ref-counted, so
reassigning |load_extension_dialog_| doesn't mean the old object is destroyed. 
Coupled with the fact that we'd have the FileHelper as the observer for both, I
feel like this could let us get into some weird state.  Are you sure that
everything can handle the possibility of multiple file dialogs being
opened/notifying of files selected?

The alternative would be to just early-return if load_extension_dialog_ already
exists, and say that we can only open one dialog at a time (of course, this
would be better if there was a way of focusing that dialog, but it doesn't look
like we have a method for that yet).

[Devlin, 2014/12/03 17:59:00]

For instance, this causes a problem on line 94, where we inform the dialog that
the listener was destroyed.  If we remove our reference to the file dialog,
while the dialog is still alive, and then the FileHelper is destroyed, we could
end up crashing because we never unregister.

+      this,
+      new ChromeSelectFilePolicy(
+          loader_handler_->web_ui()->GetWebContents()));
 
   load_extension_dialog_->SelectFile(
       kSelectType,
       title_,
       last_unpacked_directory_,
       NULL,
       kFileTypeIndex,
       base::FilePath::StringType(),
       loader_handler_->web_ui()->GetWebContents()->GetTopLevelNativeWindow(),
       NULL);
@@ skipping 183 matching lines @@
 }
 
 void ExtensionLoaderHandler::NotifyFrontendOfFailure() {
   web_ui()->CallJavascriptFunction(
       "extensions.ExtensionLoader.notifyLoadFailed",
       failures_);
   failures_.Clear();
 }
 
 }  // namespace extensions