summaryrefslogtreecommitdiffstats
path: root/chrome_frame
diff options
context:
space:
mode:
authorananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-03-15 22:14:32 +0000
committerananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-03-15 22:14:32 +0000
commitdacf0b454dc9395da3ef943b91d2bb482c530623 (patch)
tree73dcd612bc383bf056b54256e635a074522cff71 /chrome_frame
parent5ee5bef87e0d0ff6506aea55e3310e59ef767b14 (diff)
downloadchromium_src-dacf0b454dc9395da3ef943b91d2bb482c530623.zip
chromium_src-dacf0b454dc9395da3ef943b91d2bb482c530623.tar.gz
chromium_src-dacf0b454dc9395da3ef943b91d2bb482c530623.tar.bz2
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. Attempting another approach to detect the dummy IWebBrowser2 iframe instances which mshtml creates for top level navigations which results in the IE to Chrome switch not working reliably. The location for these dummy browsers is set to about:blank. We now check for the same and ignore these. I have added comments in the code indicating why we are doing this and the cases where it could break. It should be fine for most common cases as this code is only hit when the page has a meta tag. Will revisit this. This fixes http://code.google.com/p/chromium/issues/detail?id=36825 Bug=36825 Test=Covered by unit test. Review URL: http://codereview.chromium.org/911003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41645 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome_frame')
-rw-r--r--chrome_frame/bho.cc40
-rw-r--r--chrome_frame/test/data/host_browser.html7
-rw-r--r--chrome_frame/test/test_mock_with_web_server.cc92
3 files changed, 128 insertions, 11 deletions
diff --git a/chrome_frame/bho.cc b/chrome_frame/bho.cc
index c5c8aa0..aa333b7 100644
--- a/chrome_frame/bho.cc
+++ b/chrome_frame/bho.cc
@@ -165,8 +165,44 @@ bool DocumentHasEmbeddedItems(IUnknown* browser) {
if (enumerator) {
ScopedComPtr<IUnknown> unk;
DWORD fetched = 0;
- enumerator->Next(1, unk.Receive(), &fetched);
- has_embedded_items = (fetched != 0);
+ while (!has_embedded_items &&
+ SUCCEEDED(enumerator->Next(1, unk.Receive(), &fetched))
+ && fetched) {
+ // If a top level document has embedded iframes then the theory is
+ // that first the top level document finishes loading and then the
+ // iframes load. We should only treat an embedded element as an
+ // iframe if it supports the IWebBrowser interface.
+ ScopedComPtr<IWebBrowser2> embedded_web_browser2;
+ if (SUCCEEDED(embedded_web_browser2.QueryFrom(unk))) {
+ // If we initiate a top level navigation then at times MSHTML
+ // creates a temporary IWebBrowser2 interface which basically shows
+ // up as a temporary iframe in the parent document. It is not clear
+ // as to how we can detect this. I tried the usual stuff like
+ // getting to the parent IHTMLWindow2 interface. They all end up
+ // pointing to dummy tear off interfaces owned by MSHTML.
+ // As a temporary workaround, we found that the location url in
+ // this case is about:blank. We now check for the same and don't
+ // treat it as an iframe. This should be fine in most cases as we
+ // hit this code only when the actual page has a meta tag. However
+ // this would break for cases like the initial src url for an
+ // iframe pointing to about:blank and the page then writing to it
+ // via document.write.
+ // TODO(ananta)
+ // Revisit this and come up with a better approach.
+ ScopedBstr location_url;
+ embedded_web_browser2->get_LocationURL(location_url.Receive());
+
+ std::wstring location_url_string;
+ location_url_string.assign(location_url, location_url.Length());
+
+ if (!LowerCaseEqualsASCII(location_url_string, "about:blank")) {
+ has_embedded_items = true;
+ }
+ }
+
+ fetched = 0;
+ unk.Release();
+ }
}
}
}
diff --git a/chrome_frame/test/data/host_browser.html b/chrome_frame/test/data/host_browser.html
new file mode 100644
index 0000000..b6471f6
--- /dev/null
+++ b/chrome_frame/test/data/host_browser.html
@@ -0,0 +1,7 @@
+<html>
+ <head><title>Initial Page in host browser</title>
+ <body>
+ <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..4a8f76b 100644
--- a/chrome_frame/test/test_mock_with_web_server.cc
+++ b/chrome_frame/test/test_mock_with_web_server.cc
@@ -220,6 +220,31 @@ ACTION(DoCloseWindow) {
::PostMessage(arg0, WM_SYSCOMMAND, SC_CLOSE, 0);
}
+// This function selects the address bar via the Alt+d shortcut. This is done
+// via a delayed task which executes after the delay which is passed in.
+// The subsequent operations like typing in the actual url and then hitting
+// enter to force the browser to navigate to it each execute as delayed tasks
+// timed at the delay passed in. The recommended value for delay is 2000 ms
+// to account for the time taken for the accelerator keys to be reflected back
+// from Chrome.
+ACTION_P3(TypeUrlInAddressBar, loop, url, delay) {
+ loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
+ simulate_input::SendCharA, 'd', simulate_input::ALT),
+ delay);
+
+ const unsigned long kInterval = 25;
+ int next_delay = delay + kInterval;
+
+ loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
+ simulate_input::SendStringW, url), next_delay);
+
+ next_delay = next_delay + kInterval;
+
+ loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
+ simulate_input::SendCharA, VK_RETURN, simulate_input::NONE),
+ next_delay);
+}
+
TEST(ChromeFrameTest, FullTabModeIE_DisallowedUrls) {
CloseIeAtEndOfScope last_resort_close_ie;
chrome_frame_test::TimedMsgLoop loop;
@@ -452,9 +477,7 @@ TEST_F(ChromeFrameTestWithWebServer, FLAKY_FullTabModeIE_AltD) {
EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl1)))
.WillOnce(testing::DoAll(
SetFocusToChrome(&mock),
- DelaySendChar(&loop, 1500, 'd', simulate_input::ALT),
- DelaySendString(&loop, 2000, kSubFrameUrl2),
- DelaySendChar(&loop, 2200, VK_RETURN, simulate_input::NONE)));
+ TypeUrlInAddressBar(&loop, kSubFrameUrl2, 1500)));
mock.ExpectNavigationAndSwitchSequence(kSubFrameUrl2);
EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl2)))
@@ -551,7 +574,7 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) {
// We have reached url 2 and have 1 back & 1 forward entries for url 1 & 3
// Go back to url 1 now
- mock.ExpectNavigation(kSubFrameUrl2);
+ mock.ExpectNavigationAndSwitchSequence(kSubFrameUrl2);
EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl2)))
.WillOnce(testing::IgnoreResult(testing::InvokeWithoutArgs(
CreateFunctor(ReceivePointer(mock.web_browser2_),
@@ -559,7 +582,7 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) {
// We have reached url 1 and have 0 back & 2 forward entries for url 2 & 3
// Go back to url 1 now
- mock.ExpectNavigation(kSubFrameUrl1);
+ mock.ExpectNavigationAndSwitchSequence(kSubFrameUrl1);
EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl1)))
.WillOnce(CloseBrowserMock(&mock));
EXPECT_CALL(mock, OnQuit()).WillOnce(QUIT_LOOP(loop));
@@ -826,14 +849,14 @@ TEST_F(ChromeFrameTestWithWebServer,
.WillOnce(testing::DoAll(
DelaySendMouseClick(&mock, &loop, 0, 10, 10, simulate_input::RIGHT),
SendExtendedKeysEnter(&loop, 500, VK_DOWN, 1, simulate_input::NONE)));
- mock.ExpectNavigation(kSubFrameUrl1);
+ mock.ExpectNavigationAndSwitchSequence(kSubFrameUrl1);
// Go forward using Rt-Click + DOWN + DOWN + ENTER
EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl1)))
.WillOnce(testing::DoAll(
DelaySendMouseClick(&mock, &loop, 0, 10, 10, simulate_input::RIGHT),
SendExtendedKeysEnter(&loop, 500, VK_DOWN, 2, simulate_input::NONE)));
- mock.ExpectNavigation(kSubFrameUrl2);
+ mock.ExpectNavigationAndSwitchSequence(kSubFrameUrl2);
EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl2)))
.WillOnce(CloseBrowserMock(&mock));
@@ -1064,15 +1087,16 @@ TEST_F(ChromeFrameTestWithWebServer,
SetFocusToChrome(&mock),
DelaySendScanCode(&loop, 500, bkspace, simulate_input::NONE)));
- mock.ExpectNavigation(kSubFrameUrl1);
+ mock.ExpectNavigationAndSwitchSequence(kSubFrameUrl1);
EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl1)))
.WillOnce(testing::DoAll(
SetFocusToChrome(&mock),
DelaySendScanCode(&loop, 1500, bkspace, simulate_input::SHIFT)));
- mock.ExpectNavigation(kSubFrameUrl2);
+ mock.ExpectNavigationAndSwitchSequence(kSubFrameUrl2);
EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl2)))
.WillOnce(CloseBrowserMock(&mock));
+
EXPECT_CALL(mock, OnQuit()).WillOnce(QUIT_LOOP(loop));
HRESULT hr = mock.LaunchIEAndNavigate(kSubFrameUrl1);
@@ -1128,3 +1152,53 @@ 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,
+ FLAKY_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, 1500));
+
+ EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kHostBrowserUrl)))
+ .Times(0);
+
+ 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);
+}
+
generated by cgit v1.1 at 2024-09-18 23:10:20 +0000