[Android WebView] AwContentsClient.shouldCreateWindow callback part 1.

android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc

Index: android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
diff --git a/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc b/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
index c04a00f0ce7507c79b23e7567fe2205ade3291c1..68b4626ace85e09c73d0a5c27b0030414ebb05f5 100644
--- a/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
+++ b/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
@@ -22,19 +22,89 @@ using content::InterceptNavigationDelegate;
 
 namespace {
 
+using android_webview::AwContentsIoThreadClient;
+
 base::LazyInstance<android_webview::AwResourceDispatcherHostDelegate>
     g_webview_resource_dispatcher_host_delegate = LAZY_INSTANCE_INITIALIZER;
 
-// Will unconditionally cancel this resource request.
-class CancelResourceThrottle : public content::ResourceThrottle {
+void SetOnlyAllowLoadFromCache(
+    net::URLRequest* request) {
+  int load_flags = request->load_flags();
+  load_flags &= ~(net::LOAD_BYPASS_CACHE &
+                  net::LOAD_VALIDATE_CACHE &
+                  net::LOAD_PREFERRING_CACHE);
+  load_flags |= net::LOAD_ONLY_FROM_CACHE;
+  request->set_load_flags(load_flags);
+}
+
+// May cancel this resource request based on result of Java callbacks.
+class MaybeCancelResourceThrottle : public content::ResourceThrottle {
  public:
+  MaybeCancelResourceThrottle(int child_id, int route_id)
+      : child_id_(child_id),
+        route_id_(route_id) { }
   virtual void WillStartRequest(bool* defer) OVERRIDE;
+  virtual bool ShouldCancel() = 0;
+
+  scoped_ptr<AwContentsIoThreadClient> GetIoThreadClient() {
+    return AwContentsIoThreadClient::FromID(child_id_, route_id_);
+  }
+
+ private:
+  int child_id_;
+  int route_id_;
 };
 
-void CancelResourceThrottle::WillStartRequest(bool* defer) {
-  controller()->CancelWithError(net::ERR_ACCESS_DENIED);
+void MaybeCancelResourceThrottle::WillStartRequest(bool* defer) {
+  // If there is no IO thread client set at this point, use a
+  // restrictive policy. This can happen for blocked popup
+  // windows for example.
+  if (!GetIoThreadClient() || ShouldCancel()) {

[mkosiba (inactive), 2012/11/13 17:25:11]

I'd prefer to assert that an IoThreadClient exists at this point (otherwise
you're masking potential race conditions) especially that the subsequent change
will make us block the first request until the client is set.

[benm (inactive), 2012/11/13 17:34:10]

I changed it to this as even with the "false path", we will request the initial
resource for the popup. Leaving it as a dcheck means I'd be checking in crashing
code. I suppose you could argue that adding the throttle to defer the load until
the Client is ready could come as part of this patch rather than the next one to
avoid the crash...

+    controller()->CancelWithError(net::ERR_ACCESS_DENIED);
+  }
 }
 
+class MaybeCancelContentResourceThrottle : public MaybeCancelResourceThrottle {
+ public:
+  MaybeCancelContentResourceThrottle(int child_id, int route_id)
+      : MaybeCancelResourceThrottle(child_id, route_id) { }
+
+  bool ShouldCancel() {
+    return GetIoThreadClient()->ShouldBlockContentUrls();
+  }
+};
+
+class MaybeCancelFileResourceThrottle : public MaybeCancelResourceThrottle {
+ public:
+  MaybeCancelFileResourceThrottle(int child_id, int route_id)
+      : MaybeCancelResourceThrottle(child_id, route_id) { }
+
+  bool ShouldCancel() {
+    return GetIoThreadClient()->ShouldBlockFileUrls();
+  }
+};
+
+class MaybeCancelNetworkResourceThrottle : public MaybeCancelResourceThrottle {
+ public:
+  MaybeCancelNetworkResourceThrottle(int child_id, int route_id,
+                                     net::URLRequest* request)
+      : MaybeCancelResourceThrottle(child_id, route_id),
+        request_(request) { }
+
+  bool ShouldCancel() {
+    if (GetIoThreadClient()->ShouldBlockNetworkLoads()) {
+      if (request_->url().SchemeIs(chrome::kFtpScheme)) {
+        return true;
+      }
+      SetOnlyAllowLoadFromCache(request_);
+    }
+    return false;
+  }
+
+ private:
+  net::URLRequest* request_;
+};
+
 }  // namespace
 
 namespace android_webview {
@@ -62,37 +132,26 @@ void AwResourceDispatcherHostDelegate::RequestBeginning(
     bool is_continuation_of_transferred_request,
     ScopedVector<content::ResourceThrottle>* throttles) {
 
-  scoped_ptr<AwContentsIoThreadClient> io_client =
-      AwContentsIoThreadClient::FromID(child_id, route_id);
-  DCHECK(io_client.get());
-
   // Part of implementation of WebSettings.allowContentAccess.
-  if (request->url().SchemeIs(android_webview::kContentScheme) &&
-      io_client->ShouldBlockContentUrls()) {
-    throttles->push_back(new CancelResourceThrottle);
+  if (request->url().SchemeIs(android_webview::kContentScheme)) {
+    throttles->push_back(new MaybeCancelContentResourceThrottle(
+        child_id, route_id));
   }
 
   // Part of implementation of WebSettings.allowFileAccess.
-  if (request->url().SchemeIsFile() && io_client->ShouldBlockFileUrls()) {
+  if (request->url().SchemeIsFile()) {
     const GURL& url = request->url();
     if (!url.has_path() ||
         // Application's assets and resources are always available.
         (url.path().find(android_webview::kAndroidResourcePath) != 0 &&
          url.path().find(android_webview::kAndroidAssetPath) != 0)) {
-      throttles->push_back(new CancelResourceThrottle);
+      throttles->push_back(new MaybeCancelFileResourceThrottle(
+          child_id, route_id));
     }
   }
 
-  // Part of implementation of WebSettings.blockNetworkLoads.
-  if (io_client->ShouldBlockNetworkLoads()) {
-    // Need to cancel ftp since it does not support net::LOAD_ONLY_FROM_CACHE
-    // flag, so must cancel the request if network load is blocked.
-    if (request->url().SchemeIs(chrome::kFtpScheme)) {
-      throttles->push_back(new CancelResourceThrottle);
-    } else {
-      SetOnlyAllowLoadFromCache(request);
-    }
-  }
+  throttles->push_back(new MaybeCancelNetworkResourceThrottle(
+      child_id, route_id, request));
 
   // We ignore POST requests because of BUG=155250.
   if (resource_type == ResourceType::MAIN_FRAME &&
@@ -124,14 +183,4 @@ bool AwResourceDispatcherHostDelegate::HandleExternalProtocol(const GURL& url,
   return false;
 }
 
-void AwResourceDispatcherHostDelegate::SetOnlyAllowLoadFromCache(
-    net::URLRequest* request) {
-  int load_flags = request->load_flags();
-  load_flags &= ~(net::LOAD_BYPASS_CACHE &
-                  net::LOAD_VALIDATE_CACHE &
-                  net::LOAD_PREFERRING_CACHE);
-  load_flags |= net::LOAD_ONLY_FROM_CACHE;
-  request->set_load_flags(load_flags);
-}
-
 }