When ChromeFrame switches to IE on receiving the OnHttpEquiv notification indicating the presence of a meta

tag indicating that the page is to be rendered in Chrome, we check if the notification is received for a 
site rendered in an IFrame to ensure that we don't switch in this case. This code is not reliable and we end
up assuming that a top level navigation is actually an embedded frame. 

To work around this we now maintain a pending navigation count in BeforeNavigate2, which is decremented in
NavigateComplete2 and NavigateError. We perform the check for an embedded document only if the pending navigation
count is greater than 1.

This fixes http://code.google.com/p/chromium/issues/detail?id=36825

The other change made is to add a delay of 100ms in SendString between each character to better reflect actual
timing for real user input.

Bug=36825

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

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

4 files changed, 140 insertions, 10 deletions

diff --git a/chrome_frame/bho.cc b/chrome_frame/bho.cc
index e275707..5c9fc89 100644
--- a/chrome_frame/bho.cc
+++ b/chrome_frame/bho.cc
@@ -47,7 +47,24 @@ _ATL_FUNC_INFO Bho::kBeforeNavigate2Info = {
   }
 };
 
-Bho::Bho() {
+_ATL_FUNC_INFO Bho::kNavigateComplete2Info = {
+  CC_STDCALL, VT_EMPTY, 2, {
+    VT_DISPATCH,
+    VT_VARIANT | VT_BYREF
+  }
+};
+
+_ATL_FUNC_INFO Bho::kNavigateErrorInfo = {
+  CC_STDCALL, VT_EMPTY, 5, {
+    VT_DISPATCH,
+    VT_VARIANT | VT_BYREF,
+    VT_VARIANT | VT_BYREF,
+    VT_VARIANT | VT_BYREF,
+    VT_BOOL | VT_BYREF,
+  }
+};
+
+Bho::Bho() : pending_navigation_count_(0) {
 }
 
 HRESULT Bho::FinalConstruct() {
@@ -115,10 +132,27 @@ STDMETHODIMP Bho::BeforeNavigate2(IDispatch* dispatch, VARIANT* url,
     referrer_.clear();
   }
   url_ = url->bstrVal;
+  pending_navigation_count_++;
   ProcessOptInUrls(web_browser2, url->bstrVal);
   return S_OK;
 }
 
+STDMETHODIMP_(void) Bho::NavigateComplete2(IDispatch* dispatch,
+                                            VARIANT* url) {
+  DLOG(INFO) << "In NavigateComplete2 for url:" << url->bstrVal;
+  pending_navigation_count_--;
+  DCHECK(pending_navigation_count_ >= 0);
+}
+
+STDMETHODIMP_(void) Bho::OnNavigateError(IDispatch* dispatch, VARIANT* url,
+                                         VARIANT* frame_name,
+                                         VARIANT* status_code,
+                                         VARIANT* cancel) {
+  DLOG(INFO) << "In NavigateError for url:" << url->bstrVal;
+  pending_navigation_count_--;
+  DCHECK(pending_navigation_count_ >= 0);
+}
+
 HRESULT Bho::NavigateToCurrentUrlInCF(IBrowserService* browser) {
   DCHECK(browser);
   MarkBrowserOnThreadForCFNavigation(browser);
@@ -192,7 +226,13 @@ void ClearDocumentContents(IUnknown* browser) {
 
 // Returns true if the currently loaded document in the browser has
 // any embedded items such as a frame or an iframe.
-bool DocumentHasEmbeddedItems(IUnknown* browser) {
+bool DocumentHasEmbeddedItems(IUnknown* browser,
+                              int pending_navigation_count) {
+  // For a document with embedded frames the pending navigation count will be
+  // greater than 1.
+  if (pending_navigation_count <= 1)
+    return false;
+
   bool has_embedded_items = false;
 
   ScopedComPtr<IWebBrowser2> web_browser2;
@@ -238,12 +278,12 @@ HRESULT Bho::OnHttpEquiv(IBrowserService_OnHttpEquiv_Fn original_httpequiv,
       // notification is coming from those and not the top level document.
       // The embedded items should only be created once the top level
       // doc has been created.
-      if (!DocumentHasEmbeddedItems(browser)) {
-        Bho* bho = Bho::GetCurrentThreadBhoInstance();
-        DCHECK(bho);
-        DLOG(INFO) << "Found tag in page. Marking browser." << bho->url() <<
-            StringPrintf(" tid=0x%08X", ::GetCurrentThreadId());
-        if (bho) {
+      Bho* bho = Bho::GetCurrentThreadBhoInstance();
+      DCHECK(bho);
+      if (bho) {
+        if (!DocumentHasEmbeddedItems(browser, bho->pending_navigation_count())) {
+          DLOG(INFO) << "Found tag in page. Marking browser." << bho->url() <<
+              StringPrintf(" tid=0x%08X", ::GetCurrentThreadId());
           // TODO(tommi): See if we can't figure out a cleaner way to avoid
           // this.  For some documents we can hit a problem here.  When we
           // attempt to navigate the document again in CF, mshtml can "complete"
@@ -258,7 +298,6 @@ HRESULT Bho::OnHttpEquiv(IBrowserService_OnHttpEquiv_Fn original_httpequiv,
       }
     }
   }
-
   return original_httpequiv(browser, shell_view, done, in_arg, out_arg);
 }
 
diff --git a/chrome_frame/bho.h b/chrome_frame/bho.h
index c82f000..5992746 100644
--- a/chrome_frame/bho.h
+++ b/chrome_frame/bho.h
@@ -64,6 +64,10 @@ END_COM_MAP()
 BEGIN_SINK_MAP(Bho)
   SINK_ENTRY_INFO(0, DIID_DWebBrowserEvents2, DISPID_BEFORENAVIGATE2,
                   BeforeNavigate2, &kBeforeNavigate2Info)
+  SINK_ENTRY_INFO(0, DIID_DWebBrowserEvents2, DISPID_NAVIGATECOMPLETE2,
+                  NavigateComplete2, &kNavigateComplete2Info)
+  SINK_ENTRY_INFO(0, DIID_DWebBrowserEvents2, DISPID_NAVIGATEERROR,
+                  OnNavigateError, &kNavigateErrorInfo)
 END_SINK_MAP()
 
   // Lifetime management methods
@@ -74,10 +78,17 @@ END_SINK_MAP()
 
   // IObjectWithSite
   STDMETHODIMP SetSite(IUnknown* site);
+
   STDMETHOD(BeforeNavigate2)(IDispatch* dispatch, VARIANT* url, VARIANT* flags,
       VARIANT* target_frame_name, VARIANT* post_data, VARIANT* headers,
       VARIANT_BOOL* cancel);
 
+  STDMETHOD_(void, NavigateComplete2)(IDispatch* dispatch, VARIANT* url);
+
+  STDMETHOD_(void, OnNavigateError)(IDispatch* dispatch, VARIANT* url,
+                                    VARIANT* frame_name, VARIANT* status_code,
+                                    VARIANT* cancel);
+
   HRESULT NavigateToCurrentUrlInCF(IBrowserService* browser);
 
   // mshtml sends an IOleCommandTarget::Exec of OLECMDID_HTTPEQUIV
@@ -113,6 +124,10 @@ END_SINK_MAP()
 
   static void ProcessOptInUrls(IWebBrowser2* browser, BSTR url);
 
+  int pending_navigation_count() const {
+    return pending_navigation_count_;
+  }
+
  protected:
   bool PatchProtocolHandler(const CLSID& handler_clsid);
 
@@ -122,6 +137,14 @@ END_SINK_MAP()
   static base::LazyInstance<base::ThreadLocalPointer<Bho> >
       bho_current_thread_instance_;
   static _ATL_FUNC_INFO kBeforeNavigate2Info;
+  static _ATL_FUNC_INFO kNavigateComplete2Info;
+  static _ATL_FUNC_INFO kNavigateErrorInfo;
+
+  // This variable holds the pending navigation count seen by the BHO. It is
+  // incremented in BeforeNavigate2 and decremented in NavigateComplete2.
+  // Used to determine whether there are embedded frames loading for the
+  // current document.
+  int pending_navigation_count_;
 };
 
 #endif  // CHROME_FRAME_BHO_H_
diff --git a/chrome_frame/test/data/host_browser.html b/chrome_frame/test/data/host_browser.html
new file mode 100644
index 0000000..cfd21f4
--- /dev/null
+++ b/chrome_frame/test/data/host_browser.html
@@ -0,0 +1,8 @@
+<html>
+  <head><title>Initial Page in host browser</title>
+  </head>
+  <body onLoad="test();">
+    <h2>Initial page in host browser!</h2>
+    <p>This page should have loaded in the host browser.</p>
+  </body>
+</html>
diff --git a/chrome_frame/test/test_mock_with_web_server.cc b/chrome_frame/test/test_mock_with_web_server.cc
index dd02150..83522aa 100644
--- a/chrome_frame/test/test_mock_with_web_server.cc
+++ b/chrome_frame/test/test_mock_with_web_server.cc
@@ -220,6 +220,19 @@ ACTION(DoCloseWindow) {
   ::PostMessage(arg0, WM_SYSCOMMAND, SC_CLOSE, 0);
 }
 
+ACTION_P2(TypeUrlInAddressBar, loop, url) {
+  loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
+      simulate_input::SendCharA, 'd', simulate_input::ALT),
+      1500);
+
+  loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
+      simulate_input::SendStringW, url), 2000);
+
+  loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
+      simulate_input::SendCharA, VK_RETURN, simulate_input::NONE),
+      2000);
+}
+
 TEST(ChromeFrameTest, FullTabModeIE_DisallowedUrls) {
   CloseIeAtEndOfScope last_resort_close_ie;
   chrome_frame_test::TimedMsgLoop loop;
@@ -735,7 +748,7 @@ const wchar_t kBeforeUnloadMain[] =
     L"http://localhost:1337/files/fulltab_before_unload_event_main.html";
 
 // http://code.google.com/p/chromium/issues/detail?id=37231
-TEST_F(ChromeFrameTestWithWebServer, DISABLED_FullTabModeIE_UnloadEventTest) {
+TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_UnloadEventTest) {
   CloseIeAtEndOfScope last_resort_close_ie;
   chrome_frame_test::TimedMsgLoop loop;
   ComStackObjectWithUninitialize<MockWebBrowserEventSink> mock;
@@ -1128,3 +1141,50 @@ TEST_F(ChromeFrameTestWithWebServer, FLAKY_FullTabModeIE_MenuSaveAs) {
   ASSERT_TRUE(DeleteFile(kSaveFileName));
 }
 
+const wchar_t kHostBrowserUrl[] =
+    L"http://localhost:1337/files/host_browser.html";
+
+TEST_F(ChromeFrameTestWithWebServer,
+       FullTabMode_SwitchFromIEToChromeFrame) {
+  CloseIeAtEndOfScope last_resort_close_ie;
+  chrome_frame_test::TimedMsgLoop loop;
+  ComStackObjectWithUninitialize<MockWebBrowserEventSink> mock;
+
+  EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _))
+              .Times(testing::AnyNumber());
+
+  ::testing::InSequence sequence;   // Everything in sequence
+
+  // This test performs the following steps.
+  // 1. Launches IE and navigates to
+  //    http://localhost:1337/files/back_to_ie.html, which should render in IE.
+  // 2. It then navigates to
+  //    http://localhost:1337/files/sub_frame1.html which should render in
+  //    ChromeFrame
+  EXPECT_CALL(mock, OnBeforeNavigate2(_,
+              testing::Field(&VARIANT::bstrVal,
+              testing::StrCaseEq(kHostBrowserUrl)), _, _, _, _, _));
+
+  // When we receive a navigate complete notification for the initial URL
+  // initiate a navigation to a url which should be rendered in ChromeFrame.
+  EXPECT_CALL(mock, OnNavigateComplete2(_,
+              testing::Field(&VARIANT::bstrVal,
+              testing::StrCaseEq(kHostBrowserUrl))))
+      .Times(1)
+      .WillOnce(TypeUrlInAddressBar(&loop, kSubFrameUrl1));
+
+  mock.ExpectNavigationAndSwitch(kSubFrameUrl1);
+  EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl1)))
+      .WillOnce(CloseBrowserMock(&mock));
+
+  EXPECT_CALL(mock, OnQuit()).WillOnce(QUIT_LOOP(loop));
+
+  HRESULT hr = mock.LaunchIEAndNavigate(kHostBrowserUrl);
+  ASSERT_HRESULT_SUCCEEDED(hr);
+  if (hr == S_FALSE)
+    return;
+
+  ASSERT_TRUE(mock.web_browser2() != NULL);
+  loop.RunFor(kChromeFrameLongNavigationTimeoutInSeconds * 2);
+}
+