Fixed chrome://net-export save dialog code

chrome/browser/ui/webui/net_export_ui.cc

Index: chrome/browser/ui/webui/net_export_ui.cc
diff --git a/chrome/browser/ui/webui/net_export_ui.cc b/chrome/browser/ui/webui/net_export_ui.cc
index 2bdbe3757abaf48dcc19345e6d7d345ffbda2dc1..8ac8368d50813a077127c3847534ffaed689375e 100644
--- a/chrome/browser/ui/webui/net_export_ui.cc
+++ b/chrome/browser/ui/webui/net_export_ui.cc
@@ -76,6 +76,7 @@ class NetExportMessageHandler
   void FileSelected(const base::FilePath& path,
                     int index,
                     void* params) override;
+  void FileSelectionCanceled(void* params) override;
 
  private:
   // Calls NetLogFileWriter's ProcessCommand with DO_START and DO_STOP commands.
@@ -311,18 +312,21 @@ void NetExportMessageHandler::OnExportNetLogInfoChanged(base::Value* arg) {
 
 void NetExportMessageHandler::ShowSelectFileDialog(
     const base::FilePath& default_path) {
-  DCHECK(!select_file_dialog_);
+  // User may have clicked more than once before the save dialog appears.
+  // In that case, we don't want more than one save dialog to appear.

[mmenke, 2016/10/24 16:58:58]

nit:  Don't use we in comments, as it's ambiguous ("We this class", "We the
chrome developers", "We the user", "We the television network", etc).  Maybe
something like "In that case, this is needed to prevent more than one save
dialog from appearing."

[wangyix1, 2016/10/24 19:57:12]

Done.

+  if (select_file_dialog_) {
+    return;
+  }

[mmenke, 2016/10/24 16:58:58]

nit:  No braces when the if condition and body take up one line each.

[wangyix1, 2016/10/24 19:57:12]

Done.

 
-  WebContents* webcontents = nullptr;
+  WebContents* webcontents = web_ui()->GetWebContents();
 
   select_file_dialog_ = ui::SelectFileDialog::Create(
       this, new ChromeSelectFilePolicy(webcontents));
   ui::SelectFileDialog::FileTypeInfo file_type_info;
   file_type_info.extensions = {{FILE_PATH_LITERAL("json")}};
   gfx::NativeWindow owning_window =
-      webcontents ? platform_util::GetTopLevel(webcontents->GetNativeView())
+      webcontents ? webcontents->GetTopLevelNativeWindow()
                   : nullptr;

[mmenke, 2016/10/24 16:58:58]

I don't think webcontents can actually ever be NULL?  Know this is old code, but
looks like the web_contents_ value that web_ui()->GetWebContents() returns is
set on construction of WebUIImpl, and never cleared, so could we remove that
check, while we're here?

[wangyix1, 2016/10/24 19:57:12]

Done.

-
   select_file_dialog_->SelectFile(
       ui::SelectFileDialog::SELECT_SAVEAS_FILE, base::string16(), default_path,
       &file_type_info, 0, base::FilePath::StringType(), owning_window, nullptr);
@@ -331,6 +335,7 @@ void NetExportMessageHandler::ShowSelectFileDialog(
 void NetExportMessageHandler::FileSelected(const base::FilePath& path,
                                            int index,
                                            void* params) {

[mmenke, 2016/10/24 16:58:58]

Maybe put a DCHECK(select_file_dialog_); here and in FileSelectionCanceled, for
documentation purposes?

[wangyix1, 2016/10/24 19:57:12]

Done.

+  select_file_dialog_ = nullptr;
   BrowserThread::PostTaskAndReply(
       BrowserThread::FILE_USER_BLOCKING, FROM_HERE,
       base::Bind(&net_log::NetLogFileWriter::SetUpNetExportLogPath,
@@ -343,6 +348,10 @@ void NetExportMessageHandler::FileSelected(const base::FilePath& path,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
+void NetExportMessageHandler::FileSelectionCanceled(void* params) {
+  select_file_dialog_ = nullptr;
+}
+
 }  // namespace
 
 NetExportUI::NetExportUI(content::WebUI* web_ui) : WebUIController(web_ui) {