Ensure proper ordering of app window request block/unblock

This fixes a race in app window initialization which could
cause resources to be permanently blocked from loading on
behalf of the renderer.

BUG=520750
TBR=kalman@chromium.org for tabs
TBR=nasko@chromium.org for IPC message (symbol rename only)

Review URL: https://codereview.chromium.org/1294923002

Cr-Commit-Position: refs/heads/master@{#343552}

10 files changed, 86 insertions, 14 deletions

diff --git a/extensions/browser/app_window/app_window.cc b/extensions/browser/app_window/app_window.cc
index 575062b..b6d7754 100644
--- a/extensions/browser/app_window/app_window.cc
+++ b/extensions/browser/app_window/app_window.cc
@@ -714,6 +714,11 @@ void AppWindow::WindowEventsReady() {
   SendOnWindowShownIfShown();
 }
 
+void AppWindow::NotifyRenderViewReady() {
+  if (app_window_contents_)
+    app_window_contents_->OnWindowReady();
+}
+
 void AppWindow::GetSerializedState(base::DictionaryValue* properties) const {
   DCHECK(properties);
 
diff --git a/extensions/browser/app_window/app_window.h b/extensions/browser/app_window/app_window.h
index 449a729..4a06269 100644
--- a/extensions/browser/app_window/app_window.h
+++ b/extensions/browser/app_window/app_window.h
@@ -68,6 +68,9 @@ class AppWindowContents {
   // Called in tests when the window is shown
   virtual void DispatchWindowShownForTests() const = 0;
 
+  // Called when the renderer notifies the browser that the window is ready.
+  virtual void OnWindowReady() = 0;
+
   virtual content::WebContents* GetWebContents() const = 0;
 
   virtual extensions::WindowController* GetWindowController() const = 0;
@@ -341,6 +344,10 @@ class AppWindow : public content::WebContentsDelegate,
   // app.
   void WindowEventsReady();
 
+  // Notifies the window's contents that the render view is ready and it can
+  // unblock resource requests.
+  void NotifyRenderViewReady();
+
   // Whether the app window wants to be alpha enabled.
   bool requested_alpha_enabled() const { return requested_alpha_enabled_; }
 
diff --git a/extensions/browser/app_window/app_window_contents.cc b/extensions/browser/app_window/app_window_contents.cc
index 8b95283..181e211 100644
--- a/extensions/browser/app_window/app_window_contents.cc
+++ b/extensions/browser/app_window/app_window_contents.cc
@@ -21,7 +21,8 @@
 
 namespace extensions {
 
-AppWindowContentsImpl::AppWindowContentsImpl(AppWindow* host) : host_(host) {}
+AppWindowContentsImpl::AppWindowContentsImpl(AppWindow* host)
+    : host_(host), is_blocking_requests_(false), is_window_ready_(false) {}
 
 AppWindowContentsImpl::~AppWindowContentsImpl() {}
 
@@ -84,6 +85,21 @@ void AppWindowContentsImpl::DispatchWindowShownForTests() const {
       "appWindowShownForTests", args, false));
 }
 
+void AppWindowContentsImpl::OnWindowReady() {
+  is_window_ready_ = true;
+  if (is_blocking_requests_) {
+    is_blocking_requests_ = false;
+    content::RenderFrameHost* frame = web_contents_->GetMainFrame();
+    content::BrowserThread::PostTask(
+        content::BrowserThread::IO, FROM_HERE,
+        base::Bind(
+            &content::ResourceDispatcherHost::ResumeBlockedRequestsForRoute,
+            base::Unretained(content::ResourceDispatcherHost::Get()),
+            frame->GetProcess()->GetID(),
+            frame->GetRenderViewHost()->GetRoutingID()));
+  }
+}
+
 content::WebContents* AppWindowContentsImpl::GetWebContents() const {
   return web_contents_.get();
 }
@@ -110,6 +126,10 @@ void AppWindowContentsImpl::UpdateDraggableRegions(
 void AppWindowContentsImpl::SuspendRenderFrameHost(
     content::RenderFrameHost* rfh) {
   DCHECK(rfh);
+  // Don't bother blocking requests if the renderer side is already good to go.
+  if (is_window_ready_)
+    return;
+  is_blocking_requests_ = true;
   // The ResourceDispatcherHost only accepts RenderViewHost child ids.
   // TODO(devlin): This will need to change for site isolation.
   content::BrowserThread::PostTask(
diff --git a/extensions/browser/app_window/app_window_contents.h b/extensions/browser/app_window/app_window_contents.h
index 608b987..7656a68 100644
--- a/extensions/browser/app_window/app_window_contents.h
+++ b/extensions/browser/app_window/app_window_contents.h
@@ -35,6 +35,7 @@ class AppWindowContentsImpl : public AppWindowContents,
   void NativeWindowChanged(NativeAppWindow* native_app_window) override;
   void NativeWindowClosed() override;
   void DispatchWindowShownForTests() const override;
+  void OnWindowReady() override;
   content::WebContents* GetWebContents() const override;
   WindowController* GetWindowController() const override;
 
@@ -48,6 +49,8 @@ class AppWindowContentsImpl : public AppWindowContents,
   AppWindow* host_;  // This class is owned by |host_|
   GURL url_;
   scoped_ptr<content::WebContents> web_contents_;
+  bool is_blocking_requests_;
+  bool is_window_ready_;
 
   DISALLOW_COPY_AND_ASSIGN(AppWindowContentsImpl);
 };
diff --git a/extensions/browser/app_window/test_app_window_contents.cc b/extensions/browser/app_window/test_app_window_contents.cc
index 18401e7..6cdefcb 100644
--- a/extensions/browser/app_window/test_app_window_contents.cc
+++ b/extensions/browser/app_window/test_app_window_contents.cc
@@ -32,6 +32,8 @@ void TestAppWindowContents::NativeWindowClosed() {
 void TestAppWindowContents::DispatchWindowShownForTests() const {
 }
 
+void TestAppWindowContents::OnWindowReady() {}
+
 content::WebContents* TestAppWindowContents::GetWebContents() const {
   return web_contents_.get();
 }
diff --git a/extensions/browser/app_window/test_app_window_contents.h b/extensions/browser/app_window/test_app_window_contents.h
index d52fb97..97635aa 100644
--- a/extensions/browser/app_window/test_app_window_contents.h
+++ b/extensions/browser/app_window/test_app_window_contents.h
@@ -27,6 +27,7 @@ class TestAppWindowContents : public AppWindowContents {
   void NativeWindowChanged(NativeAppWindow* native_app_window) override;
   void NativeWindowClosed() override;
   void DispatchWindowShownForTests() const override;
+  void OnWindowReady() override;
   content::WebContents* GetWebContents() const override;
   WindowController* GetWindowController() const override;
 
diff --git a/extensions/browser/io_thread_extension_message_filter.cc b/extensions/browser/io_thread_extension_message_filter.cc
index 8b0188b..ebc2406 100644
--- a/extensions/browser/io_thread_extension_message_filter.cc
+++ b/extensions/browser/io_thread_extension_message_filter.cc
@@ -4,8 +4,14 @@
 
 #include "extensions/browser/io_thread_extension_message_filter.h"
 
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_view_host.h"
 #include "content/public/browser/resource_dispatcher_host.h"
+#include "content/public/browser/web_contents.h"
+#include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/browser/extension_function_dispatcher.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/info_map.h"
@@ -16,6 +22,33 @@ using content::BrowserThread;
 
 namespace extensions {
 
+namespace {
+
+void NotifyAppWindowReadyOnUIThread(int render_process_id, int route_id) {
+  content::RenderProcessHost* process =
+      content::RenderProcessHost::FromID(render_process_id);
+  if (!process)
+    return;
+  content::BrowserContext* browser_context = process->GetBrowserContext();
+  if (!browser_context)
+    return;
+  content::RenderViewHost* rvh =
+      content::RenderViewHost::FromID(render_process_id, route_id);
+  if (!rvh)
+    return;
+  content::WebContents* contents =
+      content::WebContents::FromRenderViewHost(rvh);
+  if (!contents)
+    return;
+  AppWindowRegistry* registry = AppWindowRegistry::Get(browser_context);
+  DCHECK(registry);
+  AppWindow* window = registry->GetAppWindowForWebContents(contents);
+  if (window)
+    window->NotifyRenderViewReady();
+}
+
+}  // namespace
+
 IOThreadExtensionMessageFilter::IOThreadExtensionMessageFilter(
     int render_process_id,
     content::BrowserContext* context)
@@ -41,10 +74,9 @@ bool IOThreadExtensionMessageFilter::OnMessageReceived(
     const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(IOThreadExtensionMessageFilter, message)
+    IPC_MESSAGE_HANDLER(ExtensionHostMsg_AppWindowReady, OnAppWindowReady);
   IPC_MESSAGE_HANDLER(ExtensionHostMsg_GenerateUniqueID,
                       OnExtensionGenerateUniqueID)
-  IPC_MESSAGE_HANDLER(ExtensionHostMsg_ResumeRequests,
-                      OnExtensionResumeRequests);
   IPC_MESSAGE_HANDLER(ExtensionHostMsg_RequestForIOThread,
                       OnExtensionRequestForIOThread)
   IPC_MESSAGE_UNHANDLED(handled = false)
@@ -52,17 +84,18 @@ bool IOThreadExtensionMessageFilter::OnMessageReceived(
   return handled;
 }
 
+void IOThreadExtensionMessageFilter::OnAppWindowReady(int route_id) {
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(&NotifyAppWindowReadyOnUIThread,
+                                     render_process_id_, route_id));
+}
+
 void IOThreadExtensionMessageFilter::OnExtensionGenerateUniqueID(
     int* unique_id) {
   static int next_unique_id = 0;
   *unique_id = ++next_unique_id;
 }
 
-void IOThreadExtensionMessageFilter::OnExtensionResumeRequests(int route_id) {
-  content::ResourceDispatcherHost::Get()->ResumeBlockedRequestsForRoute(
-      render_process_id_, route_id);
-}
-
 void IOThreadExtensionMessageFilter::OnExtensionRequestForIOThread(
     int routing_id,
     const ExtensionHostMsg_Request_Params& params) {
diff --git a/extensions/browser/io_thread_extension_message_filter.h b/extensions/browser/io_thread_extension_message_filter.h
index e45a253..f6f41bb 100644
--- a/extensions/browser/io_thread_extension_message_filter.h
+++ b/extensions/browser/io_thread_extension_message_filter.h
@@ -41,8 +41,8 @@ class IOThreadExtensionMessageFilter : public content::BrowserMessageFilter {
   bool OnMessageReceived(const IPC::Message& message) override;
 
   // Message handlers on the IO thread.
+  void OnAppWindowReady(int route_id);
   void OnExtensionGenerateUniqueID(int* unique_id);
-  void OnExtensionResumeRequests(int route_id);
   void OnExtensionRequestForIOThread(
       int routing_id,
       const ExtensionHostMsg_Request_Params& params);
diff --git a/extensions/common/extension_messages.h b/extensions/common/extension_messages.h
index 8ae6d19..49aee48 100644
--- a/extensions/common/extension_messages.h
+++ b/extensions/common/extension_messages.h
@@ -730,8 +730,9 @@ IPC_MESSAGE_ROUTED0(ExtensionHostMsg_DecrementLazyKeepaliveCount)
 IPC_SYNC_MESSAGE_CONTROL0_1(ExtensionHostMsg_GenerateUniqueID,
                             int /* unique_id */)
 
-// Resumes resource requests for a newly created app window.
-IPC_MESSAGE_CONTROL1(ExtensionHostMsg_ResumeRequests, int /* route_id */)
+// Notify the browser that an app window is ready and can resume resource
+// requests.
+IPC_MESSAGE_CONTROL1(ExtensionHostMsg_AppWindowReady, int /* route_id */)
 
 // Sent by the renderer when the draggable regions are updated.
 IPC_MESSAGE_ROUTED1(ExtensionHostMsg_UpdateDraggableRegions,
diff --git a/extensions/renderer/app_window_custom_bindings.cc b/extensions/renderer/app_window_custom_bindings.cc
index 0b589d8..448a46c 100644
--- a/extensions/renderer/app_window_custom_bindings.cc
+++ b/extensions/renderer/app_window_custom_bindings.cc
@@ -104,9 +104,9 @@ void AppWindowCustomBindings::GetFrame(
   blink::WebFrame* opener = context_render_frame->GetWebFrame();
   blink::WebLocalFrame* app_web_frame = app_frame->GetWebFrame();
   app_web_frame->setOpener(opener);
-  content::RenderThread::Get()->Send(
-      new ExtensionHostMsg_ResumeRequests(
-          app_frame->GetRenderView()->GetRoutingID()));
+
+  content::RenderThread::Get()->Send(new ExtensionHostMsg_AppWindowReady(
+      app_frame->GetRenderView()->GetRoutingID()));
 
   v8::Local<v8::Value> window =
       app_web_frame->mainWorldScriptContext()->Global();