summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-06-17 01:19:04 +0000
committerananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-06-17 01:19:04 +0000
commite3a91e7d2cb359692c59b86a29453f1931f87760 (patch)
tree83d18e33d0041e888e580dee0e81b15cf3d490fc
parent584245e578427abc2bc19f224609f820ad71aca0 (diff)
downloadchromium_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.cc35
-rw-r--r--chrome_frame/chrome_active_document.h4
-rw-r--r--chrome_frame/chrome_frame_activex_base.h10
-rw-r--r--chrome_frame/test/data/chrome_frame_window_open.html4
-rw-r--r--chrome_frame/test/test_mock_with_web_server.cc3
-rw-r--r--chrome_frame/test/test_mock_with_web_server.h17
-rw-r--r--chrome_frame/test/util_unittests.cc25
-rw-r--r--chrome_frame/utils.cc63
-rw-r--r--chrome_frame/utils.h11
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_