diff options
author | ericu@google.com <ericu@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-12 20:50:11 +0000 |
---|---|---|
committer | ericu@google.com <ericu@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-12 20:50:11 +0000 |
commit | 502415277b63821cc18422e177b73de41985bec6 (patch) | |
tree | 9625e2de0cef611528ee73b2f8d43c1a58245e7f | |
parent | 15d8edb4918b3729fc72200f5c19a67409fc0180 (diff) | |
download | chromium_src-502415277b63821cc18422e177b73de41985bec6.zip chromium_src-502415277b63821cc18422e177b73de41985bec6.tar.gz chromium_src-502415277b63821cc18422e177b73de41985bec6.tar.bz2 |
Reverting due to two broken tests: FullTabModeIE_SubIFram and FullTabModeIE_UnloadEventTest.
Revert 41463 - 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
TBR=ananta@chromium.org
Review URL: http://codereview.chromium.org/877009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41477 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome_frame/bho.cc | 57 | ||||
-rw-r--r-- | chrome_frame/bho.h | 23 | ||||
-rw-r--r-- | chrome_frame/test/data/host_browser.html | 8 | ||||
-rw-r--r-- | chrome_frame/test/test_mock_with_web_server.cc | 62 |
4 files changed, 10 insertions, 140 deletions
diff --git a/chrome_frame/bho.cc b/chrome_frame/bho.cc index 5c9fc89..e275707 100644 --- a/chrome_frame/bho.cc +++ b/chrome_frame/bho.cc @@ -47,24 +47,7 @@ _ATL_FUNC_INFO Bho::kBeforeNavigate2Info = { } }; -_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) { +Bho::Bho() { } HRESULT Bho::FinalConstruct() { @@ -132,27 +115,10 @@ 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); @@ -226,13 +192,7 @@ 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, - 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 DocumentHasEmbeddedItems(IUnknown* browser) { bool has_embedded_items = false; ScopedComPtr<IWebBrowser2> web_browser2; @@ -278,12 +238,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. - 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()); + 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) { // 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" @@ -298,6 +258,7 @@ 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 5992746..c82f000 100644 --- a/chrome_frame/bho.h +++ b/chrome_frame/bho.h @@ -64,10 +64,6 @@ 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 @@ -78,17 +74,10 @@ 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 @@ -124,10 +113,6 @@ 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); @@ -137,14 +122,6 @@ 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 deleted file mode 100644 index cfd21f4..0000000 --- a/chrome_frame/test/data/host_browser.html +++ /dev/null @@ -1,8 +0,0 @@ -<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 83522aa..dd02150 100644 --- a/chrome_frame/test/test_mock_with_web_server.cc +++ b/chrome_frame/test/test_mock_with_web_server.cc @@ -220,19 +220,6 @@ 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; @@ -748,7 +735,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, FullTabModeIE_UnloadEventTest) { +TEST_F(ChromeFrameTestWithWebServer, DISABLED_FullTabModeIE_UnloadEventTest) { CloseIeAtEndOfScope last_resort_close_ie; chrome_frame_test::TimedMsgLoop loop; ComStackObjectWithUninitialize<MockWebBrowserEventSink> mock; @@ -1141,50 +1128,3 @@ 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); -} - |