First cut at UI for saving net_logs data into a temporary file on

chrome/browser/ui/webui/net_export_ui.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[mmenke, 2013/01/17 17:49:43]

nit:  2013

[ramant (doing other things), 2013/01/18 04:59:52]

Done.

+// 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 <string>
+
+#include "base/bind.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/string_util.h"
+#include "base/utf_string_conversions.h"
+#include "base/values.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/net/chrome_net_log.h"
+#include "chrome/browser/net/net_log_temp_file.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/webui/chrome_url_data_manager.h"
+#include "chrome/browser/ui/webui/chrome_web_ui_data_source.h"
+#include "chrome/common/url_constants.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_ui.h"
+#include "content/public/browser/web_ui_message_handler.h"
+#include "grit/browser_resources.h"
+
+#if defined(OS_ANDROID)
+#include "chrome/browser/android/intent_helper.h"
+#endif
+
+using content::BrowserThread;
+using content::WebContents;
+using content::WebUIMessageHandler;
+
+namespace {
+
+ChromeWebUIDataSource* CreateNetExportHTMLSource() {
+  ChromeWebUIDataSource* source =
+      new ChromeWebUIDataSource(chrome::kChromeUINetExportHost);
+
+  source->set_json_path("strings.js");
+  source->add_resource_path("net_export.js", IDR_NET_EXPORT_JS);
+  source->set_default_resource(IDR_NET_EXPORT_HTML);
+  return source;
+}
+
+// This class receives javascript messages from the renderer.
+// Note that the WebUI infrastructure runs on the UI thread, therefore all of
+// this class's public methods are expected to run on the UI thread.
+class NetExportMessageHandler
+    : public WebUIMessageHandler,
+      public base::SupportsWeakPtr<NetExportMessageHandler> {
+ public:
+  NetExportMessageHandler();
+  virtual ~NetExportMessageHandler();
+
+  // WebUIMessageHandler implementation.
+  virtual void RegisterMessages() OVERRIDE;
+
+  // Messages.
+  void OnGetExportNetLogInfo(const ListValue* list);
+  void OnStartNetLog(const ListValue* list);
+  void OnStopNetLog(const ListValue* list);
+  void OnSendNetLog(const ListValue* list);
+
+ private:
+  // Calls NetLogTempFile's ProcessCommand with DO_START and DO_STOP commands.
+  static void ProcessNetLogCommand(
+      base::WeakPtr<NetExportMessageHandler> net_export_message_handler,
+      NetLogTempFile::Command command);
+
+  // Sends net_log data via email if there is a net_log_temp_file.

[mmenke, 2013/01/17 17:49:43]

There's always a NetLogTempFile class.  May want to be a little clearer.

[ramant (doing other things), 2013/01/18 04:59:52]

Done.

+  static void SendNetLog(
+      base::WeakPtr<NetExportMessageHandler> net_export_message_handler);
+
+  // Helper function to send state/file information from NetLogTempFile.
+  static void SendExportNetLogInfo(
+      base::WeakPtr<NetExportMessageHandler> net_export_message_handler);
+
+  // Helper function that runs on UI thread to send NetLog data via email.
+  static void SendEmail(const FilePath& file_to_send);
+
+  // Helper that calls g_exportBrowserBridge.receivedData in the renderer,
+  // passing in |arg|. Takes ownership of |arg|.
+  void SendJavascriptCommand(Value* arg);
+
+  base::WeakPtrFactory<NetExportMessageHandler> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetExportMessageHandler);
+};
+
+NetExportMessageHandler::NetExportMessageHandler()
+    : ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+}
+
+NetExportMessageHandler::~NetExportMessageHandler() {
+  // Cancel any in-progress requests to collect net_log into temporary file.
+  NetLogTempFile* net_log_temp_file =
+      g_browser_process->net_log()->net_log_temp_file();

[mmenke, 2013/01/17 17:49:43]

optional:  I'm not a big fan of using g_browser_process everywhere.  How about
storing a pointer to the ChromeNetLog, or even the NetLogTempFile in
NetExportMessageHandler and then passing it around?

It doesn't actually change the thread safety characteristics at all, but I just
prefer to keep direct global accesses to a minimum, and only access it on the UI
thread.

[ramant (doing other things), 2013/01/18 04:59:52]

Done.

+  BrowserThread::PostTask(
+      BrowserThread::FILE_USER_BLOCKING,
+      FROM_HERE,
+      base::Bind(&NetLogTempFile::ProcessCommand,
+                 base::Unretained(net_log_temp_file),
+                 NetLogTempFile::DO_STOP));
+}
+
+void NetExportMessageHandler::RegisterMessages() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  web_ui()->RegisterMessageCallback(
+      "getExportNetLogInfo",
+      base::Bind(&NetExportMessageHandler::OnGetExportNetLogInfo,
+                 base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "startNetLog",
+      base::Bind(&NetExportMessageHandler::OnStartNetLog,
+                 base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "stopNetLog",
+      base::Bind(&NetExportMessageHandler::OnStopNetLog,
+                 base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "sendNetLog",
+      base::Bind(&NetExportMessageHandler::OnSendNetLog,
+                 base::Unretained(this)));
+}
+
+void NetExportMessageHandler::OnGetExportNetLogInfo(const ListValue* list) {
+  BrowserThread::PostTask(
+      BrowserThread::FILE_USER_BLOCKING,
+      FROM_HERE,
+      base::Bind(&NetExportMessageHandler::SendExportNetLogInfo,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void NetExportMessageHandler::OnStartNetLog(const ListValue* list) {
+  ProcessNetLogCommand(weak_ptr_factory_.GetWeakPtr(),
+                       NetLogTempFile::DO_START);
+}
+
+void NetExportMessageHandler::OnStopNetLog(const ListValue* list) {
+  ProcessNetLogCommand(weak_ptr_factory_.GetWeakPtr(), NetLogTempFile::DO_STOP);
+}
+
+void NetExportMessageHandler::OnSendNetLog(const ListValue* list) {
+  SendNetLog(weak_ptr_factory_.GetWeakPtr());

[mmenke, 2013/01/17 17:49:43]

optional:  I think this would be a little cleaner if you used PostTaskAndReply
here, and have SendNetLog return a FilePath.  Then can rename SendNetLog to
"GetNetLogFileName".

Could also use the same pattern for the start/stop functions, but think we don't
gain much simplicity there.

[ramant (doing other things), 2013/01/18 04:59:52]

Done.

+}
+
+// static
+void NetExportMessageHandler::ProcessNetLogCommand(
+      base::WeakPtr<NetExportMessageHandler> net_export_message_handler,
+    NetLogTempFile::Command command) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::FILE_USER_BLOCKING)) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE_USER_BLOCKING,
+        FROM_HERE,
+        base::Bind(&NetExportMessageHandler::ProcessNetLogCommand,
+                   net_export_message_handler,
+                   command));
+    return;
+  }
+
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE_USER_BLOCKING));
+  NetLogTempFile* net_log_temp_file =
+      g_browser_process->net_log()->net_log_temp_file();
+  net_log_temp_file->ProcessCommand(command);
+
+  SendExportNetLogInfo(net_export_message_handler);
+}
+
+// static
+void NetExportMessageHandler::SendNetLog(
+      base::WeakPtr<NetExportMessageHandler> net_export_message_handler) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::FILE_USER_BLOCKING)) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE_USER_BLOCKING,
+        FROM_HERE,
+        base::Bind(&NetExportMessageHandler::SendNetLog,
+                   net_export_message_handler));
+    return;
+  }
+
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE_USER_BLOCKING));
+  NetLogTempFile* net_log_temp_file =
+      g_browser_process->net_log()->net_log_temp_file();
+
+  FilePath net_export_file_path;
+  if (net_log_temp_file->GetNetLogToSend(&net_export_file_path)) {
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&NetExportMessageHandler::SendEmail, net_export_file_path));
+  }
+
+  SendExportNetLogInfo(net_export_message_handler);

[mmenke, 2013/01/17 17:49:43]

Has anything changed by calling GetNetLogToSend()?  I don't think this is
needed.

[ramant (doing other things), 2013/01/18 04:59:52]

Done.

+}
+
+// static
+void NetExportMessageHandler::SendExportNetLogInfo(
+    base::WeakPtr<NetExportMessageHandler> net_export_message_handler) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE_USER_BLOCKING));
+
+  NetLogTempFile* net_log_temp_file =
+      g_browser_process->net_log()->net_log_temp_file();
+
+  Value* value = net_log_temp_file->GetState();
+  if (!BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&NetExportMessageHandler::SendJavascriptCommand,
+                 net_export_message_handler,
+                 value))) {
+    // Failed posting the task, avoid leaking.
+    delete value;
+  }
+}
+
+// static
+void NetExportMessageHandler::SendEmail(const FilePath& file_to_send) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+#if defined(OS_ANDROID)
+  std::string email;
+  std::string subject = "net_internals_log";
+  std::string title = "test_title";
+  std::string body = "Net Internals log data";

[mmenke, 2013/01/17 17:49:43]

Suggest default values that encourage users to add some informative text.

Maybe "Issue number: " for the title, for example.

[ramant (doing other things), 2013/01/18 04:59:52]

Would appreciate if you could suggest some text.

Done.

+  FilePath::StringType file_to_attach(file_to_send.value());
+  chrome::android::SendEmail(
+      UTF8ToUTF16(email), UTF8ToUTF16(subject),
+      UTF8ToUTF16(body), UTF8ToUTF16(title), UTF8ToUTF16(file_to_attach));
+#endif
+}
+
+void NetExportMessageHandler::SendJavascriptCommand(Value* arg) {
+  scoped_ptr<Value> value(arg);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  web_ui()->CallJavascriptFunction("g_exportBrowserBridge.receivedData",
+                                   *value.get());

[mmenke, 2013/01/17 17:49:43]

Doesn't just *value work here?

[ramant (doing other things), 2013/01/18 04:59:52]

NetInternalsMessageHandler::SendJavascriptCommand does the same. Changed it to
*arg.

Done.

+}
+
+}  // 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);
+  ChromeURLDataManager::AddDataSource(profile, CreateNetExportHTMLSource());
+}