diff options
author | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-17 01:19:04 +0000 |
---|---|---|
committer | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-17 01:19:04 +0000 |
commit | e3a91e7d2cb359692c59b86a29453f1931f87760 (patch) | |
tree | 83d18e33d0041e888e580dee0e81b15cf3d490fc | |
parent | 584245e578427abc2bc19f224609f820ad71aca0 (diff) | |
download | chromium_src-e3a91e7d2cb359692c59b86a29453f1931f87760.zip chromium_src-e3a91e7d2cb359692c59b86a29453f1931f87760.tar.gz chromium_src-e3a91e7d2cb359692c59b86a29453f1931f87760.tar.bz2 |
window.open calls issued by pages within ChromeFrame would not honor the suggested dimensions and would end up
opening a default top level browser window in IE.
ChromeFrame does receive the dimensions from the external tab container when it is notified about a popup being
opened.
Fix is to honor these dimensions by passing them off in the specially crafted url containing other arguments.
Fixes bug http://code.google.com/p/chromium/issues/detail?id=42250
This fix is currently implemented for IE full tab mode only.
Bug=42250
Test=Covered by augmenting the existing window open test to also validate the window size. Added a new unit test
to test the ParseAttachExternalTabUrl helper function.
Review URL: http://codereview.chromium.org/2867007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50064 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome_frame/chrome_active_document.cc | 35 | ||||
-rw-r--r-- | chrome_frame/chrome_active_document.h | 4 | ||||
-rw-r--r-- | chrome_frame/chrome_frame_activex_base.h | 10 | ||||
-rw-r--r-- | chrome_frame/test/data/chrome_frame_window_open.html | 4 | ||||
-rw-r--r-- | chrome_frame/test/test_mock_with_web_server.cc | 3 | ||||
-rw-r--r-- | chrome_frame/test/test_mock_with_web_server.h | 17 | ||||
-rw-r--r-- | chrome_frame/test/util_unittests.cc | 25 | ||||
-rw-r--r-- | chrome_frame/utils.cc | 63 | ||||
-rw-r--r-- | chrome_frame/utils.h | 11 |
9 files changed, 159 insertions, 13 deletions
diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc index 3223edd..2b8e222 100644 --- a/chrome_frame/chrome_active_document.cc +++ b/chrome_frame/chrome_active_document.cc @@ -65,7 +65,8 @@ ChromeActiveDocument::ChromeActiveDocument() : first_navigation_(true), is_automation_client_reused_(false), popup_allowed_(false), - accelerator_table_(NULL) { + accelerator_table_(NULL), + is_new_navigation_(false) { TRACE_EVENT_BEGIN("chromeframe.createactivedocument", this, ""); url_fetcher_.set_frame_busting(false); @@ -519,6 +520,23 @@ HRESULT ChromeActiveDocument::ActiveXDocActivate(LONG verb) { } else { m_hWnd = Create(parent_window, position_rect, 0, 0, WS_EX_CLIENTEDGE); } + + ScopedComPtr<IWebBrowser2> web_browser2; + DoQueryService(SID_SWebBrowserApp, m_spClientSite, + web_browser2.Receive()); + if (web_browser2) { + if (!dimensions_.IsEmpty()) { + web_browser2->put_Width(dimensions_.width()); + web_browser2->put_Height(dimensions_.height()); + web_browser2->put_Left(dimensions_.x()); + web_browser2->put_Top(dimensions_.y()); + web_browser2->put_MenuBar(VARIANT_FALSE); + web_browser2->put_ToolBar(VARIANT_FALSE); + + dimensions_.set_height(0); + dimensions_.set_width(0); + } + } } SetObjectRects(&position_rect, &clip_rect); } @@ -976,14 +994,13 @@ bool ChromeActiveDocument::LaunchUrl(const std::wstring& url, std::string utf8_url; if (!is_new_navigation) { - WStringTokenizer tokenizer(url, L"&"); - // Skip over kChromeAttachExternalTabPrefix - tokenizer.GetNext(); - + int disposition = 0; uint64 external_tab_cookie = 0; - if (tokenizer.GetNext()) { - wchar_t* end_ptr = 0; - external_tab_cookie = _wcstoui64(tokenizer.token().c_str(), &end_ptr, 10); + + if (!ParseAttachExternalTabUrl(url, &external_tab_cookie, &dimensions_, + &disposition)) { + NOTREACHED() << "Failed to parse attach tab url:" << url; + return false; } if (external_tab_cookie == 0) { @@ -991,8 +1008,10 @@ bool ChromeActiveDocument::LaunchUrl(const std::wstring& url, return false; } + is_new_navigation_ = false; automation_client_->AttachExternalTab(external_tab_cookie); } else { + is_new_navigation_ = true; // Initiate navigation before launching chrome so that the url will be // cached and sent with launch settings. if (url_.Length()) { diff --git a/chrome_frame/chrome_active_document.h b/chrome_frame/chrome_active_document.h index 5c3d8fb..977ed90 100644 --- a/chrome_frame/chrome_active_document.h +++ b/chrome_frame/chrome_active_document.h @@ -454,6 +454,10 @@ END_EXEC_COMMAND_MAP() UrlmonUrlRequestManager::PrivacyInfo::PrivacyRecords::iterator next_privacy_record_; + // Dimensions of the window. Used only when opening popups. + gfx::Rect dimensions_; + // Set to true if the document was loaded by a window.open in chrome. + bool is_new_navigation_; public: ScopedComPtr<IOleInPlaceFrame> in_place_frame_; OLEINPLACEFRAMEINFO frame_info_; diff --git a/chrome_frame/chrome_frame_activex_base.h b/chrome_frame/chrome_frame_activex_base.h index d38e964..32b431c 100644 --- a/chrome_frame/chrome_frame_activex_base.h +++ b/chrome_frame/chrome_frame_activex_base.h @@ -444,8 +444,14 @@ END_MSG_MAP() virtual void OnAttachExternalTab(int tab_handle, const IPC::AttachExternalTabParams& params) { std::string url; - url = StringPrintf("%lsattach_external_tab&%ls&%d", kChromeProtocolPrefix, - Uint64ToWString(params.cookie).c_str(), params.disposition); + url = StringPrintf("%lsattach_external_tab&%ls&%d&%d&%d&%d&%d", + kChromeProtocolPrefix, + Uint64ToWString(params.cookie).c_str(), + params.disposition, + params.dimensions.x(), + params.dimensions.y(), + params.dimensions.width(), + params.dimensions.height()); HostNavigate(GURL(url), GURL(), params.disposition); } diff --git a/chrome_frame/test/data/chrome_frame_window_open.html b/chrome_frame/test/data/chrome_frame_window_open.html index cf96a9e..22515ed 100644 --- a/chrome_frame/test/data/chrome_frame_window_open.html +++ b/chrome_frame/test/data/chrome_frame_window_open.html @@ -18,8 +18,8 @@ function onLoad() { var new_window; function OpenPopup() { - new_window = window.open("chrome_frame_window_open_popup.html", "mywindow", - "left=10, top=10, height=100, width=100"); + new_window = window.open("chrome_frame_window_open_popup.html", "_blank", + "left=10, top=10, height=250, width=250"); } function OnKeyPress() { diff --git a/chrome_frame/test/test_mock_with_web_server.cc b/chrome_frame/test/test_mock_with_web_server.cc index 8871717..bdd5c8d 100644 --- a/chrome_frame/test/test_mock_with_web_server.cc +++ b/chrome_frame/test/test_mock_with_web_server.cc @@ -371,6 +371,7 @@ TEST_F(ChromeFrameTestWithWebServer, FLAKY_FullTabModeIE_WindowOpenInChrome) { EXPECT_CALL(new_window_mock, OnLoad(testing::StrCaseEq(kWindowOpenPopupUrl))) .WillOnce(testing::DoAll( VerifyAddressBarUrl(&new_window_mock), + ValidateWindowSize(&new_window_mock, 10, 10, 250, 250), CloseBrowserMock(&new_window_mock))); EXPECT_CALL(new_window_mock, OnQuit()) @@ -389,6 +390,8 @@ TEST_F(ChromeFrameTestWithWebServer, FLAKY_FullTabModeIE_WindowOpenInChrome) { ASSERT_TRUE(mock.web_browser2() != NULL); loop.RunFor(kChromeFrameLongNavigationTimeoutInSeconds); + + ASSERT_TRUE(new_window_mock.web_browser2() != NULL); } const wchar_t kSubFrameUrl1[] = diff --git a/chrome_frame/test/test_mock_with_web_server.h b/chrome_frame/test/test_mock_with_web_server.h index 5054ad5..99fc0fe 100644 --- a/chrome_frame/test/test_mock_with_web_server.h +++ b/chrome_frame/test/test_mock_with_web_server.h @@ -106,6 +106,23 @@ ACTION_P4(DelaySendScanCode, loop, delay, c, mod) { simulate_input::SendScanCode, c, mod), delay); } +ACTION_P5(ValidateWindowSize, mock, left, top, width, height) { + long actual_left = 0; + long actual_top = 0; + long actual_width = 0; + long actual_height = 0; + + mock->web_browser2()->get_Left(&actual_left); + mock->web_browser2()->get_Top(&actual_top); + mock->web_browser2()->get_Width(&actual_width); + mock->web_browser2()->get_Height(&actual_height); + + EXPECT_EQ(actual_left, left); + EXPECT_EQ(actual_top, top); + EXPECT_EQ(actual_width, width); + EXPECT_EQ(actual_height, height); +} + } // namespace chrome_frame_test #endif // CHROME_FRAME_TEST_MOCK_WITH_WEB_SERVER_H_ diff --git a/chrome_frame/test/util_unittests.cc b/chrome_frame/test/util_unittests.cc index 81bc752..0187f5d 100644 --- a/chrome_frame/test/util_unittests.cc +++ b/chrome_frame/test/util_unittests.cc @@ -112,3 +112,28 @@ TEST(UtilTests, GetTempInternetFiles) { FilePath path = GetIETemporaryFilesFolder(); EXPECT_FALSE(path.empty()); } + +TEST(UtilTests, ParseAttachTabUrlTest) { + std::wstring url = L"attach_external_tab&10&1&0&0&100&100"; + + uint64 cookie = 0; + gfx::Rect dimensions; + int disposition = 0; + + EXPECT_TRUE(ParseAttachExternalTabUrl(url, &cookie, &dimensions, + &disposition)); + EXPECT_EQ(10, cookie); + EXPECT_EQ(1, disposition); + EXPECT_EQ(0, dimensions.x()); + EXPECT_EQ(0, dimensions.y()); + EXPECT_EQ(100, dimensions.width()); + EXPECT_EQ(100, dimensions.height()); + + url = L"http://www.foobar.com?&10&1&0&0&100&100"; + EXPECT_FALSE(ParseAttachExternalTabUrl(url, &cookie, &dimensions, + &disposition)); + url = L"attach_external_tab&10&1"; + EXPECT_FALSE(ParseAttachExternalTabUrl(url, &cookie, &dimensions, + &disposition)); +} + diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc index 735ec6f..cf27048 100644 --- a/chrome_frame/utils.cc +++ b/chrome_frame/utils.cc @@ -16,6 +16,7 @@ #include "base/scoped_bstr_win.h" #include "base/scoped_comptr_win.h" #include "base/scoped_variant_win.h" +#include "base/string_tokenizer.h" #include "base/string_util.h" #include "base/thread_local.h" #include "base/utf_string_conversions.h" @@ -1213,3 +1214,65 @@ HRESULT ReadStream(IStream* stream, size_t size, std::string* data) { return hr; } +bool ParseAttachExternalTabUrl(const std::wstring& url, uint64* cookie, + gfx::Rect* dimensions, int* disposition) { + if (!StartsWith(url, kChromeAttachExternalTabPrefix, true)) { + DLOG(WARNING) << "Invalid url passed in:" + << url.c_str(); + return false; + } + + if (!cookie || !dimensions || !disposition) + return false; + + WStringTokenizer tokenizer(url, L"&"); + // Skip over kChromeAttachExternalTabPrefix + tokenizer.GetNext(); + + // Read the following items in order. + // 1. cookie + // 2. disposition + // 3. dimension.x + // 4. dimension.y + // 5. dimension.width + // 6. dimension.height. + if (tokenizer.GetNext()) { + wchar_t* end_ptr = 0; + *cookie = _wcstoui64(tokenizer.token().c_str(), &end_ptr, 10); + } else { + return false; + } + + if (tokenizer.GetNext()) { + *disposition = _wtoi(tokenizer.token().c_str()); + } else { + return false; + } + + if (tokenizer.GetNext()) { + dimensions->set_x(_wtoi(tokenizer.token().c_str())); + } else { + return false; + } + + if (tokenizer.GetNext()) { + dimensions->set_y(_wtoi(tokenizer.token().c_str())); + } else { + return false; + } + + if (tokenizer.GetNext()) { + dimensions->set_width(_wtoi(tokenizer.token().c_str())); + } else { + return false; + } + + if (tokenizer.GetNext()) { + dimensions->set_height(_wtoi(tokenizer.token().c_str())); + } else { + return false; + } + + return true; +} + diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h index 5f25e00..11b050d 100644 --- a/chrome_frame/utils.h +++ b/chrome_frame/utils.h @@ -18,7 +18,7 @@ #include "base/lock.h" #include "base/logging.h" #include "base/thread.h" - +#include "gfx/rect.h" #include "googleurl/src/gurl.h" // utils.h : Various utility functions and classes @@ -464,4 +464,13 @@ std::string Bscf2Str(DWORD flags); // Reads data from a stream into a string. HRESULT ReadStream(IStream* stream, size_t size, std::string* data); +// Parses the attach external tab url, which comes in from Chrome in the course +// of a window.open operation. The format of this URL is as below:- +// gcf:attach_external_tab&n1&n2&x&y&width&height +// n1 -> cookie, n2 -> disposition, x, y, width, height -> dimensions of the +// window. +// Returns true on success. +bool ParseAttachExternalTabUrl(const std::wstring& url, uint64* cookie, + gfx::Rect* dimensions, int* disposition); + #endif // CHROME_FRAME_UTILS_H_ |