When creating a tab contents for a browser in the RVHDelegateHelper, create a wrapper immediately.

BUG=100456
TEST=ExtensionApiTest.WebRequestNewTab

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

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

9 files changed, 173 insertions, 4 deletions

diff --git a/chrome/browser/extensions/extension_tab_id_map.cc b/chrome/browser/extensions/extension_tab_id_map.cc
index 63c2fe7..f695419 100644
--- a/chrome/browser/extensions/extension_tab_id_map.cc
+++ b/chrome/browser/extensions/extension_tab_id_map.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "content/browser/renderer_host/render_process_host.h"
 #include "content/browser/renderer_host/render_view_host.h"
+#include "content/browser/tab_contents/navigation_details.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_observer.h"
@@ -46,6 +47,8 @@ ExtensionTabIdMap::TabObserver::TabObserver() {
                  content::NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this, content::NOTIFICATION_TAB_PARENTED,
                  content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this, content::NOTIFICATION_RETARGETING,
+                 content::NotificationService::AllBrowserContextsAndSources());
 }
 
 ExtensionTabIdMap::TabObserver::~TabObserver() {
@@ -89,6 +92,25 @@ void ExtensionTabIdMap::TabObserver::Observe(
               tab->restore_tab_helper()->window_id().id()));
       break;
     }
+    case content::NOTIFICATION_RETARGETING: {
+      content::RetargetingDetails* retargeting_details =
+          content::Details<content::RetargetingDetails>(details).ptr();
+      TabContents* contents = retargeting_details->target_tab_contents;
+      TabContentsWrapper* tab =
+          TabContentsWrapper::GetCurrentWrapperForContents(contents);
+      if (!tab)
+        break;
+      RenderViewHost* host = tab->render_view_host();
+      BrowserThread::PostTask(
+          BrowserThread::IO, FROM_HERE,
+          base::Bind(
+              &ExtensionTabIdMap::SetTabAndWindowId,
+              base::Unretained(ExtensionTabIdMap::GetInstance()),
+              host->process()->id(), host->routing_id(),
+              tab->restore_tab_helper()->session_id().id(),
+              tab->restore_tab_helper()->window_id().id()));
+      break;
+    }
     case content::NOTIFICATION_RENDER_VIEW_HOST_DELETED: {
       RenderViewHost* host = content::Source<RenderViewHost>(source).ptr();
       BrowserThread::PostTask(
diff --git a/chrome/browser/extensions/extension_webrequest_apitest.cc b/chrome/browser/extensions/extension_webrequest_apitest.cc
index bacf4b5..7939f20 100644
--- a/chrome/browser/extensions/extension_webrequest_apitest.cc
+++ b/chrome/browser/extensions/extension_webrequest_apitest.cc
@@ -4,13 +4,20 @@
 
 #include "base/command_line.h"
 #include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_webrequest_api.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/login/login_prompt.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/browser/renderer_host/render_view_host.h"
+#include "content/browser/tab_contents/tab_contents.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "net/base/mock_host_resolver.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
 
 namespace {
 
@@ -92,3 +99,36 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
   ASSERT_TRUE(RunExtensionSubtest("webrequest", "test_blocking.html")) <<
       message_;
 }
+
+IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, WebRequestNewTab) {
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableExperimentalExtensionApis);
+
+  // Wait for the extension to set itself up and return control to us.
+  ASSERT_TRUE(RunExtensionSubtest("webrequest", "test_newTab.html"))
+      << message_;
+
+  ResultCatcher catcher;
+
+  ExtensionService* service = browser()->profile()->GetExtensionService();
+  const Extension* extension =
+      service->GetExtensionById(last_loaded_extension_id_, false);
+  GURL url = extension->GetResourceURL("newTab/a.html");
+
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // There's a link on a.html with target=_blank. Click on it to open it in a
+  // new tab.
+  WebKit::WebMouseEvent mouse_event;
+  mouse_event.type = WebKit::WebInputEvent::MouseDown;
+  mouse_event.button = WebKit::WebMouseEvent::ButtonLeft;
+  mouse_event.x = 7;
+  mouse_event.y = 7;
+  mouse_event.clickCount = 1;
+  TabContents* tab = browser()->GetSelectedTabContents();
+  tab->render_view_host()->ForwardMouseEvent(mouse_event);
+  mouse_event.type = WebKit::WebInputEvent::MouseUp;
+  tab->render_view_host()->ForwardMouseEvent(mouse_event);
+
+  ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
+}
diff --git a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc
index d2e9bce2..9533c76 100644
--- a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc
+++ b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc
@@ -263,6 +263,9 @@ TabContents* RenderViewHostDelegateViewHelper::CreateNewWindowFromTabContents(
       params.frame_name);
 
   if (new_contents) {
+    if (tab_contents->delegate())
+      tab_contents->delegate()->TabContentsCreated(new_contents);
+
     content::RetargetingDetails details;
     details.source_tab_contents = tab_contents;
     details.source_frame_id = params.opener_frame_id;
@@ -273,9 +276,6 @@ TabContents* RenderViewHostDelegateViewHelper::CreateNewWindowFromTabContents(
         content::Source<content::BrowserContext>(
             tab_contents->browser_context()),
         content::Details<content::RetargetingDetails>(&details));
-
-    if (tab_contents->delegate())
-      tab_contents->delegate()->TabContentsCreated(new_contents);
   } else {
     content::NotificationService::current()->Notify(
         content::NOTIFICATION_CREATING_NEW_WINDOW_CANCELLED,
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index ad054f5..8846965 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -3824,6 +3824,15 @@ bool Browser::ShouldAddNavigationToHistory(
   return !IsApplication();
 }
 
+void Browser::TabContentsCreated(TabContents* new_contents) {
+  // Create a TabContentsWrapper now, so all observers are in place, as the
+  // network requests for its initial navigation will start immediately. The
+  // TabContents will later be inserted into this browser using
+  // Browser::Navigate via AddNewContents. The latter will retrieve the newly
+  // created TabContentsWrapper from TabContents object.
+  new TabContentsWrapper(new_contents);
+}
+
 void Browser::ContentRestrictionsChanged(TabContents* source) {
   UpdateCommandsForContentRestrictionState();
 }
diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h
index 010e569..9c5a0623 100644
--- a/chrome/browser/ui/browser.h
+++ b/chrome/browser/ui/browser.h
@@ -976,6 +976,7 @@ class Browser : public TabHandlerDelegate,
   virtual bool ShouldAddNavigationToHistory(
       const history::HistoryAddPageArgs& add_page_args,
       content::NavigationType navigation_type) OVERRIDE;
+  virtual void TabContentsCreated(TabContents* new_contents) OVERRIDE;
   virtual void ContentRestrictionsChanged(TabContents* source) OVERRIDE;
   virtual void RendererUnresponsive(TabContents* source) OVERRIDE;
   virtual void RendererResponsive(TabContents* source) OVERRIDE;
diff --git a/chrome/test/data/extensions/api_test/webrequest/framework.js b/chrome/test/data/extensions/api_test/webrequest/framework.js
index 7a13b11..f7e2157 100644
--- a/chrome/test/data/extensions/api_test/webrequest/framework.js
+++ b/chrome/test/data/extensions/api_test/webrequest/framework.js
@@ -8,6 +8,7 @@ var expectedEventData;
 var capturedEventData;
 var expectedEventOrder;
 var tabId;
+var tabIdMap;
 var testServerPort;
 var testServer = "www.a.com";
 var eventsCaptured;
@@ -15,6 +16,8 @@ var eventsCaptured;
 function runTests(tests) {
   chrome.tabs.create({url: "about:blank"}, function(tab) {
     tabId = tab.id;
+    tabIdMap = {};
+    tabIdMap[tabId] = 0;
     chrome.test.getConfig(function(config) {
       testServerPort = config.testServer.port;
       chrome.test.runTests(tests);
@@ -71,7 +74,7 @@ function expect(data, order, filter, extraInfoSpec) {
       expectedEventData[i].details.method = "GET";
     }
     if (!expectedEventData[i].details.tabId) {
-      expectedEventData[i].details.tabId = tabId;
+      expectedEventData[i].details.tabId = tabIdMap[tabId];
     }
     if (!expectedEventData[i].details.type) {
       expectedEventData[i].details.type = "main_frame";
@@ -161,6 +164,13 @@ function captureEvent(name, details, callback) {
   }
   delete details.frameId;
 
+  // This assigns unique IDs to newly opened tabs. However, the new IDs are only
+  // deterministic, if the order in which the tabs are opened is deterministic.
+  if (!(details.tabId in tabIdMap)) {
+    tabIdMap[details.tabId] = Object.keys(tabIdMap).length;
+  }
+  details.tabId = tabIdMap[details.tabId];
+
   delete details.requestId;
   delete details.timeStamp;
   if (details.requestHeaders) {
diff --git a/chrome/test/data/extensions/api_test/webrequest/newTab/a.html b/chrome/test/data/extensions/api_test/webrequest/newTab/a.html
new file mode 100644
index 0000000..332d419
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/webrequest/newTab/a.html
@@ -0,0 +1,11 @@
+<html><head>
+<style type="text/css">
+body {
+  margin: 0;
+}
+</style>
+</head>
+<body>
+<a href="b.html" target="_blank">link</a>
+</body>
+</html>
diff --git a/chrome/test/data/extensions/api_test/webrequest/newTab/b.html b/chrome/test/data/extensions/api_test/webrequest/newTab/b.html
new file mode 100644
index 0000000..18ecdcb
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/webrequest/newTab/b.html
@@ -0,0 +1 @@
+<html></html>
diff --git a/chrome/test/data/extensions/api_test/webrequest/test_newTab.html b/chrome/test/data/extensions/api_test/webrequest/test_newTab.html
new file mode 100644
index 0000000..b813374
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/webrequest/test_newTab.html
@@ -0,0 +1,75 @@
+<script src="framework.js">
+</script>
+<script>
+runTests([
+  // Navigates to a page with a link with target=_blank. Then simulates a click
+  // on that link and verifies that the new tab has a correct tab ID assigned.
+  function () {
+    expect(
+      [  // events
+        { label: "a-onBeforeRequest",
+          event: "onBeforeRequest",
+          details: {
+            url: getURL("newTab/a.html"),
+            frameUrl: getURL("newTab/a.html")
+          }
+        },
+        { label: "a-onResponseStarted",
+          event: "onResponseStarted",
+          details: {
+            url: getURL("newTab/a.html"),
+            statusCode: 200,
+            fromCache: false,
+            statusLine: "HTTP/1.1 200 OK",
+            // Request to chrome-extension:// url has no IP.
+          }
+        },
+        { label: "a-onCompleted",
+          event: "onCompleted",
+          details: {
+            url: getURL("newTab/a.html"),
+            statusCode: 200,
+            fromCache: false,
+            statusLine: "HTTP/1.1 200 OK",
+            // Request to chrome-extension:// url has no IP.
+          }
+        },
+        { label: "b-onBeforeRequest",
+          event: "onBeforeRequest",
+          details: {
+            url: getURL("newTab/b.html"),
+            frameUrl: getURL("newTab/b.html"),
+            tabId: 1,
+          }
+        },
+        { label: "b-onResponseStarted",
+          event: "onResponseStarted",
+          details: {
+            url: getURL("newTab/b.html"),
+            statusCode: 200,
+            fromCache: false,
+            statusLine: "HTTP/1.1 200 OK",
+            // Request to chrome-extension:// url has no IP.
+            tabId: 1,
+          }
+        },
+        { label: "b-onCompleted",
+          event: "onCompleted",
+          details: {
+            url: getURL("newTab/b.html"),
+            statusCode: 200,
+            fromCache: false,
+            statusLine: "HTTP/1.1 200 OK",
+            // Request to chrome-extension:// url has no IP.
+            tabId: 1,
+          }
+        },
+      ],
+      [  // event order
+      ["a-onBeforeRequest", "a-onResponseStarted", "a-onCompleted",
+       "b-onBeforeRequest", "b-onResponseStarted", "b-onCompleted"] ]);
+    // Notify the api test that we're waiting for the user.
+    chrome.test.notifyPass();
+  },
+]);
+</script>