Avoid sync IPCs for FileSystem API

chrome/renderer/content_settings_observer.cc

Index: chrome/renderer/content_settings_observer.cc
diff --git a/chrome/renderer/content_settings_observer.cc b/chrome/renderer/content_settings_observer.cc
index f5489a07f70d2ccaa399358f20f14dfad50041af..0e0012977f05c9f70483b1c92d8ff1c7f247d3cb 100644
--- a/chrome/renderer/content_settings_observer.cc
+++ b/chrome/renderer/content_settings_observer.cc
@@ -10,11 +10,13 @@
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/renderer/extensions/dispatcher.h"
+#include "chrome/renderer/permission_request_map.h"
 #include "content/public/renderer/document_state.h"
 #include "content/public/renderer/navigation_state.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_view.h"
 #include "extensions/common/constants.h"
+#include "third_party/WebKit/public/platform/WebPermissionCallbacks.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
 #include "third_party/WebKit/public/web/WebDataSource.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
@@ -28,6 +30,7 @@ using blink::WebDataSource;
 using blink::WebDocument;
 using blink::WebFrame;
 using blink::WebFrameClient;
+using blink::WebPermissionCallbacks;
 using blink::WebSecurityOrigin;
 using blink::WebString;
 using blink::WebURL;
@@ -194,6 +197,8 @@ bool ContentSettingsObserver::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ChromeViewMsg_SetAllowRunningInsecureContent,
                         OnSetAllowRunningInsecureContent)
     IPC_MESSAGE_HANDLER(ChromeViewMsg_ReloadFrame, OnReloadFrame);
+    IPC_MESSAGE_HANDLER(ChromeViewMsg_RequestFileSystemAccess_ACK,
+                        OnRequestFileSystemAccessACK)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   if (handled)
@@ -261,6 +266,19 @@ bool ContentSettingsObserver::allowFileSystem(WebFrame* frame) {
   return result;
 }
 
+void ContentSettingsObserver::requestFileSystemAccess(
+    WebFrame* frame, const WebPermissionCallbacks& callbacks) {
+  int request_id =
+      content::PermissionRequestMap::GetOrCreate()->
+          RegisterPermissionCallbacks(callbacks);

[kinuko, 2014/03/05 08:35:44]

If you don't call this on worker thread you don't need to use thread-local map,
ContentSettingsObserver can just have a map as a member variable.  Also I think
in your case the map could be a simple std::map<int,
blink::WebPermissionCallback>, then no extra new/scoped_ptr<> would be
necessary.

[Fady Samuel, 2014/03/06 18:38:25]

Done.

+
+  Send(new ChromeViewHostMsg_RequestFileSystemAccess(
+      routing_id(),
+      request_id,
+      GURL(frame->document().securityOrigin().toString()),
+      GURL(frame->top()->document().securityOrigin().toString())));
+}
+
 bool ContentSettingsObserver::allowImage(WebFrame* frame,
                                          bool enabled_per_settings,
                                          const WebURL& image_url) {
@@ -589,6 +607,18 @@ void ContentSettingsObserver::OnReloadFrame() {
   render_frame()->GetWebFrame()->reload();
 }
 
+void ContentSettingsObserver::OnRequestFileSystemAccessACK(int request_id,
+                                                           bool allowed) {
+  scoped_ptr<WebPermissionCallbacks> callbacks(
+      content::PermissionRequestMap::Get()->GetAndUnregisterCallbacks(
+          request_id));
+  if (allowed) {
+    callbacks->doAllow();
+  } else {
+    callbacks->doDeny();
+  }

[kinuko, 2014/03/05 08:35:44]

nit: no need of { } for one-line body

[Fady Samuel, 2014/03/06 18:38:25]

I prefer having them when the if statement has an else clause (I don't think if
we explicitly disallow this in the style guide). My reasoning is if we replace
the one line with a nested if in the future, the else clause doesn't
accidentally get attached to the wrong thing later. I replaced else with an
early exit.

+}
+
 void ContentSettingsObserver::ClearBlockedContentSettings() {
   for (size_t i = 0; i < arraysize(content_blocked_); ++i)
     content_blocked_[i] = false;