ChromeFrame new window tests fail consistently on the IE8 builder because we expect the NewWindow3

event to be fired when a new window is opened. IE8 version 8.0.7601.17514 on the builder seems
to be firing the NewWindow2 event consistently which breaks the expectations of these tests.

Added supports in the NewWindow2 handler in the event sink to call the OnNewBrowserWindow function
which most of the tests expect.

A limitation of this approach is we don't see the url in the NewWindow2 event. ChromeFrame tests
at this point don't seem to be using the url parameter.

BUG=none
TEST=ChromeFrame new window tests should pass on the IE8 builder.
TBR=amit
Review URL: http://codereview.chromium.org/6792004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@80246 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 30 insertions, 4 deletions

diff --git a/chrome_frame/test/ie_event_sink.cc b/chrome_frame/test/ie_event_sink.cc
index 639c6f8..993d43c 100644
--- a/chrome_frame/test/ie_event_sink.cc
+++ b/chrome_frame/test/ie_event_sink.cc
@@ -431,10 +431,31 @@ STDMETHODIMP_(void) IEEventSink::OnDownloadBegin() {
     listener_->OnDownloadBegin();
 }
 
-STDMETHODIMP_(void) IEEventSink::OnNewWindow2(IDispatch** disp,
+STDMETHODIMP_(void) IEEventSink::OnNewWindow2(IDispatch** dispatch,
                                               VARIANT_BOOL* s) {
+  DVLOG(1) << __FUNCTION__;
+
+  EXPECT_TRUE(dispatch);
+  if (!dispatch)
+    return;
+
   if (listener_)
-    listener_->OnNewWindow2(disp, s);
+    listener_->OnNewWindow2(dispatch, s);
+
+  // Note that |dispatch| is an [in/out] argument. IE is asking listeners if
+  // they want to use a IWebBrowser2 of their choice for the new window.
+  // Since we need to listen on events on the new browser, we create one
+  // if needed.
+  if (!*dispatch) {
+    ScopedComPtr<IDispatch> new_browser;
+    HRESULT hr = new_browser.CreateInstance(CLSID_InternetExplorer, NULL,
+                                            CLSCTX_LOCAL_SERVER);
+    DCHECK(SUCCEEDED(hr) && new_browser);
+    *dispatch = new_browser.Detach();
+  }
+
+  if (*dispatch && listener_)
+    listener_->OnNewBrowserWindow(*dispatch, ScopedBstr());
 }
 
 STDMETHODIMP_(void) IEEventSink::OnNavigateError(IDispatch* dispatch,
@@ -511,7 +532,7 @@ STDMETHODIMP_(void) IEEventSink::OnNewWindow3(
     *dispatch = new_browser.Detach();
   }
 
-  if (*dispatch)
+  if (*dispatch && listener_)
     listener_->OnNewBrowserWindow(*dispatch, url);
 }
 
diff --git a/chrome_frame/test/mock_ie_event_sink_test.cc b/chrome_frame/test/mock_ie_event_sink_test.cc
index 6eb6622..99ccfc4 100644
--- a/chrome_frame/test/mock_ie_event_sink_test.cc
+++ b/chrome_frame/test/mock_ie_event_sink_test.cc
@@ -136,7 +136,12 @@ void MockIEEventSink::ExpectJavascriptWindowOpenNavigation(
 void MockIEEventSink::ExpectNewWindow(MockIEEventSink* new_window_mock) {
   DCHECK(new_window_mock);
 
-  EXPECT_CALL(*this, OnNewWindow3(_, _, _, _, _));
+  // IE8 seems to fire one of these events based on version.
+  EXPECT_CALL(*this, OnNewWindow2(_, _))
+      .Times(testing::AtMost(1));
+  EXPECT_CALL(*this, OnNewWindow3(_, _, _, _, _))
+      .Times(testing::AtMost(1));
+
   EXPECT_CALL(*this, OnNewBrowserWindow(_, _))
       .WillOnce(testing::WithArgs<0>(testing::Invoke(testing::CreateFunctor(
           new_window_mock, &MockIEEventSink::Attach))));