Change Web Intents delivery sequence to avoid race of page render

and intent payload delivery.

R=jam@chromium.org
BUG=112268
TEST=


Review URL: http://codereview.chromium.org/9358004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@121173 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 11 insertions, 34 deletions

diff --git a/content/browser/intents/intent_injector.cc b/content/browser/intents/intent_injector.cc
index 7bab732..869cc2b 100644
--- a/content/browser/intents/intent_injector.cc
+++ b/content/browser/intents/intent_injector.cc
@@ -44,27 +44,9 @@ void IntentInjector::SetIntent(
     const webkit_glue::WebIntentData& intent) {
   intents_dispatcher_ = intents_dispatcher;
   source_intent_.reset(new webkit_glue::WebIntentData(intent));
-
-  SendIntent();
-}
-
-void IntentInjector::RenderViewCreated(RenderViewHost* host) {
-  SendIntent();
-}
-
-void IntentInjector::DidNavigateMainFrame(
-    const content::LoadCommittedDetails& details,
-    const content::FrameNavigateParams& params) {
-  SendIntent();
 }
 
-// TODO(gbillock): The "correct" thing here is for this to be a
-// RenderViewHostObserver, and do this on RenderViewHostInitialized. There's no
-// good hooks for attaching the intent to such an object, though. All RVHOs get
-// made deep inside tab contents initialization. Idea: propagate out
-// RenderViewHostInitialized to a WebContentsObserver latch? That still looks
-// like it might be racy, though.
-void IntentInjector::SendIntent() {
+void IntentInjector::RenderViewCreated(RenderViewHost* render_view_host) {
   if (source_intent_.get() == NULL ||
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableWebIntents) ||
@@ -72,11 +54,8 @@ void IntentInjector::SendIntent() {
     return;
   }
 
-  // Send intent data through to renderer.
-  web_contents()->GetRenderViewHost()->Send(new IntentsMsg_SetWebIntentData(
-      web_contents()->GetRenderViewHost()->routing_id(),
-      *(source_intent_.get())));
-  source_intent_.reset(NULL);
+  render_view_host->Send(new IntentsMsg_SetWebIntentData(
+      render_view_host->routing_id(), *(source_intent_.get())));
 }
 
 bool IntentInjector::OnMessageReceived(const IPC::Message& message) {
diff --git a/content/browser/intents/intent_injector.h b/content/browser/intents/intent_injector.h
index e5e9896..b6cf613 100644
--- a/content/browser/intents/intent_injector.h
+++ b/content/browser/intents/intent_injector.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -26,6 +26,11 @@ struct WebIntentData;
 // the context of the service, which will be running in the TabContents on which
 // this class is an observer. Attaches to the service tab and deletes itself
 // when that TabContents is closed.
+//
+// This object should be attached to the new WebContents very early: before the
+// RenderView is created. It will then send the intent data down to the renderer
+// on the RenderViewCreated call, so that the intent data is available
+// throughout the parsing of the loaded document.
 class CONTENT_EXPORT IntentInjector : public content::WebContentsObserver {
  public:
   // |web_contents| must not be NULL.
@@ -33,10 +38,7 @@ class CONTENT_EXPORT IntentInjector : public content::WebContentsObserver {
   virtual ~IntentInjector();
 
   // content::WebContentsObserver implementation.
-  virtual void RenderViewCreated(RenderViewHost* host) OVERRIDE;
-  virtual void DidNavigateMainFrame(
-      const content::LoadCommittedDetails& details,
-      const content::FrameNavigateParams& params) OVERRIDE;
+  virtual void RenderViewCreated(RenderViewHost* render_view_host) OVERRIDE;
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
   virtual void WebContentsDestroyed(content::WebContents* tab) OVERRIDE;
 
@@ -53,9 +55,6 @@ class CONTENT_EXPORT IntentInjector : public content::WebContentsObserver {
                  const webkit_glue::WebIntentData& intent);
 
  private:
-  // Delivers the intent data to the renderer.
-  void SendIntent();
-
   // Handles receiving a reply from the intent delivery page.
   void OnReply(webkit_glue::WebIntentReplyType reply_type,
                const string16& data);
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index e8c24e7..31f47e1 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -474,8 +474,6 @@ RenderViewImpl::RenderViewImpl(
     decrement_shared_popup_at_destruction_ = false;
   }
 
-  intents_host_ = new WebIntentsHost(this);
-
   RenderThread::Get()->AddRoute(routing_id_, this);
   // Take a reference on behalf of the RenderThread.  This will be balanced
   // when we receive ViewMsg_ClosePage.
@@ -519,6 +517,7 @@ RenderViewImpl::RenderViewImpl(
   devtools_agent_ = new DevToolsAgent(this);
   renderer_accessibility_ = new RendererAccessibility(this);
   mouse_lock_dispatcher_ = new MouseLockDispatcher(this);
+  intents_host_ = new WebIntentsHost(this);
 
   new IdleUserDetector(this);