Fix a case where non-tabbed browser windows could open tabs. Also includes UI...

Fix a case where non-tabbed browser windows could open tabs. Also includes UI test for this.

http://crbug.com/8472


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

[ananta, 2009-03-31 01:18:50]

Please check if you need to update the reference build for chrome.

[sky, 2009-03-31 15:39:25]

http://codereview.chromium.org/56094/diff/1/6
File chrome/browser/automation/automation_provider.cc (right):

http://codereview.chromium.org/56094/diff/1/6#newcode1007
Line 1007: Send(reply_message);
How come you send the response immediately? If append_tab_response is initially
>= 0, then the response isn't sent immediately, instead by way of the
TabStripObserver. Perhaps you need to change the AddTabStripObserver call if you
end up with a different browser?

http://codereview.chromium.org/56094/diff/1/6#newcode2108
Line 2108: // TODO(???): implement testing for app frames.
how about a NOTREACHED here

http://codereview.chromium.org/56094/diff/1/8
File chrome/browser/browser_uitest.cc (right):

http://codereview.chromium.org/56094/diff/1/8#newcode313
Line 313: ASSERT_TRUE(normal_browser->ApplyAccelerator(IDC_EXIT));
You shouldn't need this. UITest should close all browsers when done.

[sky, 2009-03-31 18:59:24]

LGTM

[Ben Goodger (Google), 2009-03-31 23:28:01]

Hi Scott,

Here's the updated version with an in process browser test.

Ananta, Since it no longer uses the automation API no reference build changes
are required.

[sky, 2009-03-31 23:29:57]

LGTM

http://codereview.chromium.org/56094/diff/25/29
File chrome/browser/browser_unittest.cc (right):

http://codereview.chromium.org/56094/diff/25/29#newcode24
Line 24: // This test loads a page with frames and starts FindInPage requests
Something tells me you don't mean FindInPage here;)