PlzNavigate: fix timing issue in app window creation

This CL fixes a timing bug in which a newly created app window would try to
register a ScriptContext before being told to commit a navigation. This would
result in an incorrect frame url being passed to the ScriptContextSet, and a
wrong ContextType to be computed from it. With this CL, when browser-side
navigation is enabled, sending the response to the app.window.create is
deferred until the navigation in the newly created frame is ready to commit.

BUG=504347

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

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

5 files changed, 51 insertions, 0 deletions

diff --git a/extensions/browser/api/app_window/app_window_api.cc b/extensions/browser/api/app_window/app_window_api.cc
index 2cd0710..7124192 100644
--- a/extensions/browser/api/app_window/app_window_api.cc
+++ b/extensions/browser/api/app_window/app_window_api.cc
@@ -14,6 +14,7 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/browser/app_window/app_window_client.h"
@@ -367,6 +368,16 @@ bool AppWindowCreateFunction::RunAsync() {
     return true;
   }
 
+  // PlzNavigate: delay sending the response until the newly created window has
+  // been told to navigate, and blink has been correctly initialized in the
+  // renderer.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          ::switches::kEnableBrowserSideNavigation)) {
+    app_window->SetOnFirstCommitCallback(
+        base::Bind(&AppWindowCreateFunction::SendResponse, this, true));
+    return true;
+  }
+
   SendResponse(true);
   app_window->WindowEventsReady();
 
diff --git a/extensions/browser/app_window/app_window.cc b/extensions/browser/app_window/app_window.cc
index b2bfeb0..0f24847 100644
--- a/extensions/browser/app_window/app_window.cc
+++ b/extensions/browser/app_window/app_window.cc
@@ -8,9 +8,12 @@
 #include <string>
 #include <vector>
 
+#include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task_runner.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "content/public/browser/browser_context.h"
@@ -442,6 +445,26 @@ void AppWindow::DidFirstVisuallyNonEmptyPaint() {
   }
 }
 
+void AppWindow::SetOnFirstCommitCallback(const base::Closure& callback) {
+  DCHECK(on_first_commit_callback_.is_null());
+  on_first_commit_callback_ = callback;
+}
+
+void AppWindow::OnReadyToCommitFirstNavigation() {
+  CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
+      ::switches::kEnableBrowserSideNavigation));
+  WindowEventsReady();
+  if (on_first_commit_callback_.is_null())
+    return;
+  // It is important that the callback executes after the calls to
+  // WebContentsObserver::ReadyToCommitNavigation have been processed. The
+  // CommitNavigation IPC that will properly set up the renderer will only be
+  // sent after these, and it must be sent before the callback gets to run,
+  // hence the use of PostTask.
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::ResetAndReturn(&on_first_commit_callback_));
+}
+
 void AppWindow::OnNativeClose() {
   AppWindowRegistry::Get(browser_context_)->RemoveAppWindow(this);
   if (app_window_contents_) {
diff --git a/extensions/browser/app_window/app_window.h b/extensions/browser/app_window/app_window.h
index f6c330d..13ff3c3 100644
--- a/extensions/browser/app_window/app_window.h
+++ b/extensions/browser/app_window/app_window.h
@@ -242,6 +242,13 @@ class AppWindow : public content::WebContentsDelegate,
   // is on startup and from within UpdateWindowTitle().
   base::string16 GetTitle() const;
 
+  // |callback| will then be called when the first navigation in the window is
+  // ready to commit.
+  void SetOnFirstCommitCallback(const base::Closure& callback);
+
+  // Called when the first navigation in the window is ready to commit.
+  void OnReadyToCommitFirstNavigation();
+
   // Call to notify ShellRegistry and delete the window. Subclasses should
   // invoke this method instead of using "delete this".
   void OnNativeClose();
@@ -551,6 +558,9 @@ class AppWindow : public content::WebContentsDelegate,
   // Whether |is_ime_window| was set in the CreateParams.
   bool is_ime_window_;
 
+  // PlzNavigate: this is called when the first navigation is ready to commit.
+  base::Closure on_first_commit_callback_;
+
   base::WeakPtrFactory<AppWindow> image_loader_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(AppWindow);
diff --git a/extensions/browser/app_window/app_window_contents.cc b/extensions/browser/app_window/app_window_contents.cc
index 181e211..0362998 100644
--- a/extensions/browser/app_window/app_window_contents.cc
+++ b/extensions/browser/app_window/app_window_contents.cc
@@ -118,6 +118,12 @@ bool AppWindowContentsImpl::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
+void AppWindowContentsImpl::ReadyToCommitNavigation(
+    content::NavigationHandle* handle) {
+  if (!is_window_ready_)
+    host_->OnReadyToCommitFirstNavigation();
+}
+
 void AppWindowContentsImpl::UpdateDraggableRegions(
     const std::vector<DraggableRegion>& regions) {
   host_->UpdateDraggableRegions(regions);
diff --git a/extensions/browser/app_window/app_window_contents.h b/extensions/browser/app_window/app_window_contents.h
index 7656a68..559bd92 100644
--- a/extensions/browser/app_window/app_window_contents.h
+++ b/extensions/browser/app_window/app_window_contents.h
@@ -42,6 +42,7 @@ class AppWindowContentsImpl : public AppWindowContents,
  private:
   // content::WebContentsObserver
   bool OnMessageReceived(const IPC::Message& message) override;
+  void ReadyToCommitNavigation(content::NavigationHandle* handle) override;
 
   void UpdateDraggableRegions(const std::vector<DraggableRegion>& regions);
   void SuspendRenderFrameHost(content::RenderFrameHost* rfh);