Fix a few test failures when landing http://trac.webkit.org/changeset/57178....

Fix a few test failures when landing http://trac.webkit.org/changeset/57178.
http://trac.webkit.org/changeset/57178 broke some tests in browser_test and ui_test.

In the test RedirectTest.ClientCancelled, it needs a user-initiated event to trigger the redirect, now it uses "javaScript:click()". When landing r57178, the call of window.open which is inside the call of javaScript:click() was treated as non user-initiated, so the in-page location change was treated as client redirect and the redirect was recoreded, that is why this test was failed when landing r57178. (Please refer to the logic in FrameLoaderClientImpl::dispatchDidNavigateWithinPage.)

In the tests AppApiTest.AppProcess, ExtensionBrowserTest.WindowOpenExtension and ExtensionBrowserTest.WindowOpenInvalidExtension, they assume the new tabs opened by window.open in current window. But when landing r57178, since those calls of window.open were treated as non user-initiated, the disposition type of new tabs were changed to Popup, which caused a few new tabs were created instead of a few new tabs in current window (Please refer to RenderView::show), which cause those tests were failed when landing r57178.), that is why those tests were failed when landing r57178.

BUG=17655
TEST=RedirectTest.ClientCancelled, AppApiTest.AppProcess, ExtensionBrowserTest.WindowOpenExtension, ExtensionBrowserTest.WindowOpenInvalidExtension


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=56834

[Matt Perry, 2010-08-18 19:34:12]

Cool, thanks for fixing these! LGTM with a few small comments addressed.

http://codereview.chromium.org/3136019/diff/1/2
File chrome/browser/extensions/app_process_apitest.cc (right):

http://codereview.chromium.org/3136019/diff/1/2#newcode25
chrome/browser/extensions/app_process_apitest.cc:25: bool
newtab_process_should_equal_with_opener) {
nit: drop the 'with'

http://codereview.chromium.org/3136019/diff/1/2#newcode38
chrome/browser/extensions/app_process_apitest.cc:38:
EXPECT_NE(last_active_browser, static_cast<Browser*>(NULL));
use EXPECT_TRUE(last_active_browser)

http://codereview.chromium.org/3136019/diff/1/2#newcode40
chrome/browser/extensions/app_process_apitest.cc:40: EXPECT_NE(newtab,
static_cast<TabContents*>(NULL));
ditto

http://codereview.chromium.org/3136019/diff/1/3
File chrome/browser/extensions/extension_browsertests_misc.cc (right):

http://codereview.chromium.org/3136019/diff/1/3#newcode620
chrome/browser/extensions/extension_browsertests_misc.cc:620:
EXPECT_NE(last_active_browser, static_cast<Browser*>(NULL));
EXPECT_TRUE

http://codereview.chromium.org/3136019/diff/1/3#newcode622
chrome/browser/extensions/extension_browsertests_misc.cc:622: EXPECT_NE(newtab,
static_cast<TabContents*>(NULL));
ditto

http://codereview.chromium.org/3136019/diff/1/4
File chrome/browser/history/redirect_uitest.cc (right):

http://codereview.chromium.org/3136019/diff/1/4#newcode131
chrome/browser/history/redirect_uitest.cc:131:
ASSERT_TRUE(window->SimulateOSKeyPress(base::VKEY_D, 0));
where does this keypress get sent? to the URL bar?

[Johnny(Jianning) Ding, 2010-08-19 06:40:58]

http://codereview.chromium.org/3136019/diff/1/2
File chrome/browser/extensions/app_process_apitest.cc (right):

http://codereview.chromium.org/3136019/diff/1/2#newcode25
chrome/browser/extensions/app_process_apitest.cc:25: bool
newtab_process_should_equal_with_opener) {
On 2010/08/18 19:34:12, Matt Perry wrote:
> nit: drop the 'with'

Done.

http://codereview.chromium.org/3136019/diff/1/2#newcode38
chrome/browser/extensions/app_process_apitest.cc:38:
EXPECT_NE(last_active_browser, static_cast<Browser*>(NULL));
On 2010/08/18 19:34:12, Matt Perry wrote:
> use EXPECT_TRUE(last_active_browser)

Done.

http://codereview.chromium.org/3136019/diff/1/2#newcode40
chrome/browser/extensions/app_process_apitest.cc:40: EXPECT_NE(newtab,
static_cast<TabContents*>(NULL));
On 2010/08/18 19:34:12, Matt Perry wrote:
> ditto

Done.

http://codereview.chromium.org/3136019/diff/1/3
File chrome/browser/extensions/extension_browsertests_misc.cc (right):

http://codereview.chromium.org/3136019/diff/1/3#newcode620
chrome/browser/extensions/extension_browsertests_misc.cc:620:
EXPECT_NE(last_active_browser, static_cast<Browser*>(NULL));
On 2010/08/18 19:34:12, Matt Perry wrote:
> EXPECT_TRUE

Done.

http://codereview.chromium.org/3136019/diff/1/3#newcode622
chrome/browser/extensions/extension_browsertests_misc.cc:622: EXPECT_NE(newtab,
static_cast<TabContents*>(NULL));
On 2010/08/18 19:34:12, Matt Perry wrote:
> ditto

Done.

http://codereview.chromium.org/3136019/diff/1/4
File chrome/browser/history/redirect_uitest.cc (right):

http://codereview.chromium.org/3136019/diff/1/4#newcode131
chrome/browser/history/redirect_uitest.cc:131:
ASSERT_TRUE(window->SimulateOSKeyPress(base::VKEY_D, 0));
On 2010/08/18 19:34:12, Matt Perry wrote:
> where does this keypress get sent? to the URL bar?
I expect that the key should be sent to tab view container window, but the
behavior was flaky, sometimes passed, sometimes failed.
Now I use the SimulateOSClick, but the function is broken on the Mac:
http://crbug.com/45162. Now only enable this test on Windows and Linux.

[Matt Perry, 2010-08-19 18:19:19]

OK, LGTM

http://codereview.chromium.org/3136019/diff/8001/9001
File chrome/browser/extensions/app_process_apitest.cc (right):

http://codereview.chromium.org/3136019/diff/8001/9001#newcode100
chrome/browser/extensions/app_process_apitest.cc:100:
ASSERT_EQ(static_cast<size_t>(1),
you can use 1u

[Johnny(Jianning) Ding, 2010-08-20 06:21:36]

Yes, yes! Thanks!
On 2010/08/19 18:19:19, Matt Perry wrote:
> OK, LGTM
> 
> http://codereview.chromium.org/3136019/diff/8001/9001
> File chrome/browser/extensions/app_process_apitest.cc (right):
> 
> http://codereview.chromium.org/3136019/diff/8001/9001#newcode100
> chrome/browser/extensions/app_process_apitest.cc:100:
> ASSERT_EQ(static_cast<size_t>(1),
> you can use 1u