diff options
author | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-22 01:01:32 +0000 |
---|---|---|
committer | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-22 01:01:32 +0000 |
commit | d0ed50d2803d910f8e2b9f5451ee1f80c0d340ed (patch) | |
tree | e042e400647fc33f20f95318a74f7e136665fb08 | |
parent | fe7f3aa2e93a5d6634a2d81c29ac0f71e46c54db (diff) | |
download | chromium_src-d0ed50d2803d910f8e2b9f5451ee1f80c0d340ed.zip chromium_src-d0ed50d2803d910f8e2b9f5451ee1f80c0d340ed.tar.gz chromium_src-d0ed50d2803d910f8e2b9f5451ee1f80c0d340ed.tar.bz2 |
In pages rendered in ChromeFrame window open requests or link clicks with target blank which target a different origin
should initiate the navigation in the host browser. We achieve this by performing an origin check on the opener frame
and the URL being opened. If the origins don't match we allow the host browser to handle this navigation.
There is still one issue here as a popup window creation request is still initiated and sent out to the host browser
which initiates a dummy attach external tab navigation. Subsequently while applying policy the OpenURL IPC is sent out
to the host browser which initiates the navigation to the expected URL. This causes a dummy attach external tab entry
to be created in the host browser's history which would have to be deleted.
Fixes bug http://code.google.com/p/chromium/issues/detail?id=46667
Bug=46667
Test=Covered by new chrome frame unit test.
Review URL: http://codereview.chromium.org/2855017
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50416 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/external_tab_container_win.cc | 14 | ||||
-rw-r--r-- | chrome/browser/external_tab_container_win.h | 4 | ||||
-rwxr-xr-x | chrome/renderer/render_view.cc | 13 | ||||
-rw-r--r-- | chrome/test/automation/automation_proxy_uitest.cc | 10 | ||||
-rw-r--r-- | chrome_frame/test/data/chrome_frame_target_blank.html | 46 | ||||
-rw-r--r-- | chrome_frame/test/test_mock_with_web_server.cc | 61 |
6 files changed, 131 insertions, 17 deletions
diff --git a/chrome/browser/external_tab_container_win.cc b/chrome/browser/external_tab_container_win.cc index 94aec46..4e22b3e 100644 --- a/chrome/browser/external_tab_container_win.cc +++ b/chrome/browser/external_tab_container_win.cc @@ -37,7 +37,10 @@ static const wchar_t kWindowObjectKey[] = L"ChromeWindowObject"; -ExternalTabContainer::PendingTabs ExternalTabContainer::pending_tabs_; +base::LazyInstance<ExternalTabContainer::PendingTabs> + ExternalTabContainer::pending_tabs_(base::LINKER_INITIALIZED); + +// ExternalTabContainer::PendingTabs ExternalTabContainer::pending_tabs_; ExternalTabContainer* ExternalTabContainer::innermost_tab_for_unload_event_ = NULL; @@ -347,7 +350,7 @@ void ExternalTabContainer::AddNewContents(TabContents* source, if (result) { uintptr_t cookie = reinterpret_cast<uintptr_t>(new_container.get()); - pending_tabs_[cookie] = new_container; + pending_tabs_.Get()[cookie] = new_container; new_container->set_pending(true); IPC::AttachExternalTabParams attach_params_; attach_params_.cookie = static_cast<uint64>(cookie); @@ -764,10 +767,11 @@ bool ExternalTabContainer::InitNavigationInfo(IPC::NavigationInfo* nav_info, scoped_refptr<ExternalTabContainer> ExternalTabContainer::RemovePendingTab( uintptr_t cookie) { - PendingTabs::iterator index = pending_tabs_.find(cookie); - if (index != pending_tabs_.end()) { + ExternalTabContainer::PendingTabs& pending_tabs = pending_tabs_.Get(); + PendingTabs::iterator index = pending_tabs.find(cookie); + if (index != pending_tabs.end()) { scoped_refptr<ExternalTabContainer> container = (*index).second; - pending_tabs_.erase(index); + pending_tabs.erase(index); return container; } diff --git a/chrome/browser/external_tab_container_win.h b/chrome/browser/external_tab_container_win.h index 133be98..6306749 100644 --- a/chrome/browser/external_tab_container_win.h +++ b/chrome/browser/external_tab_container_win.h @@ -7,7 +7,7 @@ #include <vector> #include <map> - +#include "base/lazy_instance.h" #include "chrome/browser/automation/automation_resource_message_filter.h" #include "chrome/browser/automation/automation_profile_impl.h" #include "chrome/browser/browser.h" @@ -292,7 +292,7 @@ class ExternalTabContainer : public TabContentsDelegate, scoped_ptr<Browser> browser_; // Contains ExternalTabContainers that have not been connected to as yet. - static PendingTabs pending_tabs_; + static base::LazyInstance<PendingTabs> pending_tabs_; // True if this tab is currently the conduit for extension API automation. bool enabled_extension_automation_; diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc index 693746f..5d5079f 100755 --- a/chrome/renderer/render_view.cc +++ b/chrome/renderer/render_view.cc @@ -5172,6 +5172,19 @@ bool RenderView::IsNonLocalTopLevelNavigation( return true; } } + // Not interested in reloads. + if (type != WebKit::WebNavigationTypeReload && + type != WebKit::WebNavigationTypeFormSubmitted) { + // The opener relationship between the new window and the parent allows the + // new window to script the parent and vice versa. This is not allowed if + // the origins of the two domains are different. This can be treated as a + // top level navigation and routed back to the host. + WebKit::WebFrame* opener = frame->opener(); + if (opener) { + if (url.GetOrigin() != GURL(opener->url()).GetOrigin()) + return true; + } + } return false; } diff --git a/chrome/test/automation/automation_proxy_uitest.cc b/chrome/test/automation/automation_proxy_uitest.cc index cf037ed..1629147 100644 --- a/chrome/test/automation/automation_proxy_uitest.cc +++ b/chrome/test/automation/automation_proxy_uitest.cc @@ -1281,10 +1281,6 @@ TEST_F(ExternalTabUITestPopupEnabled, UserGestureTargetBlank) { "<a href='http://foo.com/' target='_blank'>Link</a>"; mock_->ServeHTMLData(1, main_url, main_html); - GURL foo_url("http://foo.com/"); - std::string foo_html = "<!DOCTYPE html>Foo lives here"; - mock_->ServeHTMLData(2, foo_url, foo_html); - HWND foo_host = CreateWindowW(L"Button", L"foo_host", WS_OVERLAPPEDWINDOW | WS_CLIPCHILDREN, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, NULL, NULL, NULL, NULL); @@ -1296,15 +1292,11 @@ TEST_F(ExternalTabUITestPopupEnabled, UserGestureTargetBlank) { EXPECT_CALL(*mock_, OnAttachExternalTab(1, _)) .Times(1) - .WillOnce(testing::WithArgs<1>(testing::Invoke(CreateFunctor(mock_, - &ExternalTabUITestMockClient::ConnectToExternalTab, foo_host)))); - - EXPECT_CALL(*mock_, OnLoad(2, _)).WillOnce(QUIT_LOOP_SOON(&loop, 500)); + .WillOnce(QUIT_LOOP_SOON(&loop, 500)); mock_->CreateTabWithUrl(main_url); loop.RunFor(action_max_timeout_ms()); - EXPECT_CALL(*mock_, HandleClosed(2)); EXPECT_CALL(*mock_, HandleClosed(1)); ::DestroyWindow(foo_host); mock_->DestroyHostWindow(); diff --git a/chrome_frame/test/data/chrome_frame_target_blank.html b/chrome_frame/test/data/chrome_frame_target_blank.html new file mode 100644 index 0000000..5247e8e --- /dev/null +++ b/chrome_frame/test/data/chrome_frame_target_blank.html @@ -0,0 +1,46 @@ +<!-- saved from url=(0014)about:internet --> +<!-- Note that for the above Mark of the Web to work, the comment must + be followed by a CR/LF ending, so please do not change the line endings. --> +<html> +<!-- This page is meant to load inside a host browser like IE/FF --> +<head> +<meta http-equiv="X-UA-Compatible" content="chrome=1"/> +<script type="text/javascript" src="chrome_frame_tester_helpers.js"></script> +<script type="text/javascript"> + +function onLoad() { + if (!TestIfRunningInChrome()) { + onFailure("ChromeFrameWindowOpen", "Window Open failed :-(", + "User agent = " + navigator.userAgent.toLowerCase()); + } +} + +var new_window; + +function OpenPopup() { + new_window = window.open("http://www.nonexistent.com", "_blank", + "left=10, top=10, height=250, width=250"); +} + +function OnKeyPress() { + var char_code = String.fromCharCode(event.keyCode); + if (char_code == 'O') { + OpenPopup(); + } else if (char_code == 'C') { + new_window.close(); + } +} + +</script> +</head> + +<body onload="onLoad();" onkeypress="OnKeyPress();"> +<div id="statusPanel" style="border: 1px solid red; width: 100%"> +ChromeFrame full tab mode window open test running....<br /> +This test validates that a window open request with target blank targeted<br /> +at a different domain routes the navigation back to the host browser. +</div> +<p> + +</body> +</html>
\ No newline at end of file diff --git a/chrome_frame/test/test_mock_with_web_server.cc b/chrome_frame/test/test_mock_with_web_server.cc index d27691c..0611a9d 100644 --- a/chrome_frame/test/test_mock_with_web_server.cc +++ b/chrome_frame/test/test_mock_with_web_server.cc @@ -1311,7 +1311,6 @@ TEST(IEPrivacy, NavigationToRestrictedSite) { ProtocolPatchMethod patch_method = GetPatchMethod(); - //testing::InSequence s; const wchar_t* url = L"http://localhost:1337/files/meta_tag.html"; const wchar_t* kDialogClass = L"#32770"; @@ -1582,3 +1581,63 @@ TEST_F(ChromeFrameTestWithWebServer, FLAKY_FullTabModeIE_WindowCloseInChrome) { loop.RunFor(kChromeFrameLongNavigationTimeoutInSeconds); } +const wchar_t kWindowOpenTargetBlankUrl[] = + L"http://localhost:1337/files/chrome_frame_target_blank.html"; + +// This test checks if window.open calls with target blank issued for a +// different domain make it back to IE instead of completing the navigation +// within Chrome. We validate this by initiating a navigation to a non existent +// url which ensures we would get an error during navigation. +// Marking this test as FLAKY initially as it relies on getting focus and user +// input which don't work correctly at times. +// http://code.google.com/p/chromium/issues/detail?id=26549 +TEST_F(ChromeFrameTestWithWebServer, + FLAKY_FullTabModeIE_WindowOpenTargetBlankInChrome) { + CloseIeAtEndOfScope last_resort_close_ie; + ComStackObjectWithUninitialize<MockWebBrowserEventSink> mock; + ComStackObjectWithUninitialize<MockWebBrowserEventSink> new_window_mock; + chrome_frame_test::TimedMsgLoop loop; + + mock.ExpectNavigationAndSwitch(kWindowOpenTargetBlankUrl); + + EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kWindowOpenTargetBlankUrl))) + .WillOnce(testing::DoAll( + DelaySendMouseClick(&mock, &loop, 0, 10, 10, simulate_input::LEFT), + DelaySendChar(&loop, 500, 'O', simulate_input::NONE))); + // Watch for new window + mock.ExpectNewWindow(&new_window_mock); + + EXPECT_CALL(new_window_mock, OnBeforeNavigate2(_, + testing::Field(&VARIANT::bstrVal, + testing::HasSubstr(L"attach_external_tab")), _, _, _, _, _)) + .Times(testing::AtMost(1)); + + EXPECT_CALL(new_window_mock, OnBeforeNavigate2(_, + testing::Field(&VARIANT::bstrVal, + testing::StrCaseEq(kHostBrowserUrl)), _, _, _, _, _)) + .Times(testing::AtMost(1)); + + EXPECT_CALL(new_window_mock, OnNavigateError(_, _, _, _, _)) + .Times(1) + .WillOnce(CloseBrowserMock(&new_window_mock)); + + EXPECT_CALL(new_window_mock, OnQuit()) + .Times(1) + .WillOnce(CloseBrowserMock(&mock)); + + EXPECT_CALL(mock, OnQuit()) + .Times(1) + .WillOnce(QUIT_LOOP(loop)); + + HRESULT hr = mock.LaunchIEAndNavigate(kWindowOpenTargetBlankUrl); + ASSERT_HRESULT_SUCCEEDED(hr); + if (hr == S_FALSE) + return; + + ASSERT_TRUE(mock.web_browser2() != NULL); + + loop.RunFor(kChromeFrameLongNavigationTimeoutInSeconds * 2); + + ASSERT_TRUE(new_window_mock.web_browser2() != NULL); +} + |