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

4 files changed, 47 insertions, 2 deletions

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));
+}
+