[fsp] Show request logs in chrome://provided-file-systems.

chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc

Index: chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc
diff --git a/chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc b/chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc
index 1dbbe441617f878e428336f5cb50d070f9bf6b98..9037654457e5ef654e8ca52581de5212362b5f5e 100644
--- a/chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc
+++ b/chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc
@@ -4,9 +4,11 @@
 
 #include "chrome/browser/ui/webui/chromeos/provided_file_systems_ui.h"
 
+#include <string>
 #include <vector>
 
 #include "base/bind.h"
+#include "base/files/file.h"
 #include "base/memory/weak_ptr.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/file_system_provider/provided_file_system_info.h"
@@ -29,11 +31,24 @@ namespace chromeos {
 namespace {
 
 // Class to handle messages from chrome://provided-file-systems.
-class ProvidedFileSystemsWebUIHandler : public content::WebUIMessageHandler {
+class ProvidedFileSystemsWebUIHandler
+    : public content::WebUIMessageHandler,
+      public file_system_provider::RequestManager::Observer {
  public:
   ProvidedFileSystemsWebUIHandler() : weak_ptr_factory_(this) {}
 
-  virtual ~ProvidedFileSystemsWebUIHandler() {}
+  virtual ~ProvidedFileSystemsWebUIHandler();
+
+  // RequestManager::Observer overrides.
+  virtual void OnRequestCreated(
+      int request_id,
+      file_system_provider::RequestType type) OVERRIDE;
+  virtual void OnRequestDestroyed(int request_id) OVERRIDE;
+  virtual void OnRequestExecuted(int request_id) OVERRIDE;
+  virtual void OnRequestFulfilled(int request_id, bool has_more) OVERRIDE;
+  virtual void OnRequestRejected(int request_id,
+                                 base::File::Error error) OVERRIDE;
+  virtual void OnRequestTimeouted(int request_id) OVERRIDE;
 
  private:
   // content::WebUIMessageHandler overrides.
@@ -44,17 +59,102 @@ class ProvidedFileSystemsWebUIHandler : public content::WebUIMessageHandler {
   file_system_provider::Service* GetService();
 
   // Invoked when updating file system list is requested.
-  void OnUpdateFileSystems(const base::ListValue* args);
+  void UpdateFileSystems(const base::ListValue* args);
+
+  // Invoked when a file system is selected from the list.
+  void SelectFileSystem(const base::ListValue* args);
 
+  std::string selected_extension_id;
+  std::string selected_file_system_id;
   base::WeakPtrFactory<ProvidedFileSystemsWebUIHandler> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ProvidedFileSystemsWebUIHandler);
 };
 
+ProvidedFileSystemsWebUIHandler::~ProvidedFileSystemsWebUIHandler() {
+  // Stop observing the currently selected file system.
+  file_system_provider::Service* const service = GetService();
+  if (!service)
+    return;
+
+  file_system_provider::ProvidedFileSystemInterface* const file_system =
+      service->GetProvidedFileSystem(selected_extension_id,
+                                     selected_file_system_id);
+
+  if (file_system) {
+    file_system_provider::RequestManager* const request_manager =
+        file_system->GetRequestManager();
+    DCHECK(request_manager);
+    request_manager->RemoveObserver(this);

[Nikita (slow), 2014/05/30 07:15:10]

nit: Just file_system->GetRequestManager()->RemoveObserver(this);

[mtomasz, 2014/06/02 03:16:24]

This doesn't happen now, but theoretically ProvidedFileSystemInterface allows to
return NULL here, so I'd prefer to have at least a DCHECK here.

[Nikita (slow), 2014/06/02 06:15:12]

Either way it will crash if you try to access it but using DCHECK is fine too.

+  }
+}
+
+void ProvidedFileSystemsWebUIHandler::OnRequestCreated(
+    int request_id,
+    file_system_provider::RequestType type) {
+  base::DictionaryValue event;
+  event.SetInteger("id", request_id);

[Nikita (slow), 2014/05/30 07:15:10]

nit: Please extract all IDs as constants, they're used in several methods.

[mtomasz, 2014/06/02 03:16:24]

Done.

+  event.SetString("eventType", "created");
+  event.SetString("requestType",
+                  file_system_provider::RequestTypeToString(type));
+  event.SetDouble("time", base::Time::Now().ToJsTime());
+  web_ui()->CallJavascriptFunction("onRequestEvent", event);
+}
+
+void ProvidedFileSystemsWebUIHandler::OnRequestDestroyed(int request_id) {
+  base::DictionaryValue event;
+  event.SetInteger("id", request_id);

[Nikita (slow), 2014/05/30 07:15:10]

nit: You may introduce helper function that will fill in common fields like
id/eventType/time. And may as well call onRequestEvent JS function.

Prior to calling that function you may fill in additional data such as
hasMore/error etc.

[mtomasz, 2014/06/02 03:16:24]

Done.

+  event.SetString("eventType", "destroyed");
+  event.SetDouble("time", base::Time::Now().ToJsTime());
+  web_ui()->CallJavascriptFunction("onRequestEvent", event);
+}
+
+void ProvidedFileSystemsWebUIHandler::OnRequestExecuted(int request_id) {
+  base::DictionaryValue event;
+  event.SetInteger("id", request_id);
+  event.SetString("eventType", "executed");
+  event.SetDouble("time", base::Time::Now().ToJsTime());
+  web_ui()->CallJavascriptFunction("onRequestEvent", event);
+}
+
+void ProvidedFileSystemsWebUIHandler::OnRequestFulfilled(int request_id,
+                                                         bool has_more) {
+  base::DictionaryValue event;
+  event.SetString("eventType", "fulfilled");
+  event.SetInteger("id", request_id);
+  event.SetDouble("time", base::Time::Now().ToJsTime());
+  event.SetBoolean("hasMore", has_more);
+  web_ui()->CallJavascriptFunction("onRequestEvent", event);
+}
+
+void ProvidedFileSystemsWebUIHandler::OnRequestRejected(
+    int request_id,
+    base::File::Error error) {
+  base::DictionaryValue event;
+  event.SetInteger("id", request_id);
+  event.SetString("eventType", "rejected");
+  event.SetDouble("time", base::Time::Now().ToJsTime());
+  event.SetString("error", base::File::ErrorToString(error));
+
+  web_ui()->CallJavascriptFunction("onRequestEvent", event);
+}
+
+void ProvidedFileSystemsWebUIHandler::OnRequestTimeouted(int request_id) {
+  base::DictionaryValue event;
+  event.SetInteger("id", request_id);
+  event.SetString("eventType", "timeouted");
+  event.SetDouble("time", base::Time::Now().ToJsTime());
+  web_ui()->CallJavascriptFunction("onRequestEvent", event);
+}
+
 void ProvidedFileSystemsWebUIHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "updateFileSystems",
-      base::Bind(&ProvidedFileSystemsWebUIHandler::OnUpdateFileSystems,
+      base::Bind(&ProvidedFileSystemsWebUIHandler::UpdateFileSystems,
+                 weak_ptr_factory_.GetWeakPtr()));
+  web_ui()->RegisterMessageCallback(
+      "selectFileSystem",
+      base::Bind(&ProvidedFileSystemsWebUIHandler::SelectFileSystem,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
@@ -65,7 +165,7 @@ file_system_provider::Service* ProvidedFileSystemsWebUIHandler::GetService() {
   return file_system_provider::ServiceFactory::FindExisting(profile);
 }
 
-void ProvidedFileSystemsWebUIHandler::OnUpdateFileSystems(
+void ProvidedFileSystemsWebUIHandler::UpdateFileSystems(
     const base::ListValue* args) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -91,21 +191,64 @@ void ProvidedFileSystemsWebUIHandler::OnUpdateFileSystems(
         file_system->GetRequestManager();
     DCHECK(request_manager);
 
-    base::DictionaryValue* item_value = new base::DictionaryValue();
-    item_value->SetString("id", file_system_info.file_system_id());
-    item_value->SetString("name", file_system_info.file_system_name());
-    item_value->SetString("extensionId", file_system_info.extension_id());
-    item_value->SetString("mountPath",
-                          file_system_info.mount_path().AsUTF8Unsafe());
-    item_value->SetInteger("activeRequests",
-                           request_manager->GetActiveRequestsForLogging());
+    base::DictionaryValue* item = new base::DictionaryValue();
+    item->SetString("id", file_system_info.file_system_id());
+    item->SetString("name", file_system_info.file_system_name());
+    item->SetString("extensionId", file_system_info.extension_id());
+    item->SetString("mountPath", file_system_info.mount_path().AsUTF8Unsafe());
+    item->SetInteger("activeRequests",
+                     request_manager->GetActiveRequestsForLogging());
 
-    items.Append(item_value);
+    items.Append(item);
   }
 
   web_ui()->CallJavascriptFunction("updateFileSystems", items);
 }
 
+void ProvidedFileSystemsWebUIHandler::SelectFileSystem(
+    const base::ListValue* args) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  file_system_provider::Service* const service = GetService();
+  if (!service)
+    return;
+
+  std::string extension_id;
+  if (!args->GetString(0, &extension_id))
+    return;
+
+  std::string file_system_id;
+  if (!args->GetString(1, &file_system_id))
+    return;
+
+  // Stop observing the previously selected request manager.
+  {
+    file_system_provider::ProvidedFileSystemInterface* const file_system =
+        service->GetProvidedFileSystem(selected_extension_id,
+                                       selected_file_system_id);
+    if (file_system) {
+      file_system_provider::RequestManager* const request_manager =

[Nikita (slow), 2014/05/30 07:15:10]

nit: file_system->GetRequestManager()->RemoveObserver(this);

[mtomasz, 2014/06/02 03:16:24]

As above.

+          file_system->GetRequestManager();
+      DCHECK(request_manager);
+      request_manager->RemoveObserver(this);
+    }
+  }
+
+  // Observe the selected file system.
+  file_system_provider::ProvidedFileSystemInterface* const file_system =
+      service->GetProvidedFileSystem(extension_id, file_system_id);
+  if (!file_system)
+    return;
+
+  file_system_provider::RequestManager* const request_manager =

[Nikita (slow), 2014/05/30 07:15:10]

nit: same here.

[mtomasz, 2014/06/02 03:16:24]

As above.

+      file_system->GetRequestManager();
+  DCHECK(request_manager);
+
+  request_manager->AddObserver(this);
+  selected_extension_id = extension_id;
+  selected_file_system_id = file_system_id;
+}
+
 }  // namespace
 
 ProvidedFileSystemsUI::ProvidedFileSystemsUI(content::WebUI* web_ui)