Fixed chrome://net-export save dialog code

chrome/browser/ui/webui/net_export_ui.cc

 // Copyright (c) 2013 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/net_export_ui.h"
 
 #include <memory>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 58 matching lines @@
   // Messages.
   void OnGetExportNetLogInfo(const base::ListValue* list);
   void OnStartNetLog(const base::ListValue* list);
   void OnStopNetLog(const base::ListValue* list);
   void OnSendNetLog(const base::ListValue* list);
 
   // ui::SelectFileDialog::Listener:
   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.
   static void ProcessNetLogCommand(
       base::WeakPtr<NetExportMessageHandler> net_export_message_handler,
       net_log::NetLogFileWriter* net_log_file_writer,
       net_log::NetLogFileWriter::Command command);
 
   // Returns the path to the file which has NetLog data.
   static base::FilePath GetNetLogFileName(
@@ skipping 215 matching lines @@
 
 void NetExportMessageHandler::OnExportNetLogInfoChanged(base::Value* arg) {
   std::unique_ptr<base::Value> value(arg);
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   web_ui()->CallJavascriptFunctionUnsafe(net_log::kOnExportNetLogInfoChanged,
                                          *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);
 }
 
 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,
                  base::Unretained(net_log_file_writer_), path),
       // NetExportMessageHandler is tied to the lifetime of the tab
       // so it cannot be assured that it will be valid when this
       // StartNetLog is called. Instead of using base::Unretained a
       // weak pointer is used to adjust for this.
       base::Bind(&NetExportMessageHandler::StartNetLog,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
+void NetExportMessageHandler::FileSelectionCanceled(void* params) {
+  select_file_dialog_ = nullptr;
+}
+
 }  // namespace
 
 NetExportUI::NetExportUI(content::WebUI* web_ui) : WebUIController(web_ui) {
   web_ui->AddMessageHandler(new NetExportMessageHandler());
 
   // Set up the chrome://net-export/ source.
   Profile* profile = Profile::FromWebUI(web_ui);
   content::WebUIDataSource::Add(profile, CreateNetExportHTMLSource());
 }