Restore closed tabs into new windows when necessary, and track the windows th...

Restore closed tabs into new windows when necessary, and track the windows they
came from so they're restored together (into the same new window) when
appropriate.

Fix safety check on tab index when restoring: make it check the correct
browser.

Change some ASSERTs to EXPECTs in the unit test for greater coverage.

BUG=5278
TEST=Open a window with two tabs, close both (closing the window), then
  restore both. Make sure both restored tabs are in the same window.  Open
  a window with multiple tabs, close a tab, then close the window using 
  its close box. Restore both and make sure the tab goes back into the window.

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

[sky, 2009-04-22 14:45:04]

http://codereview.chromium.org/92001/diff/19/1006
File chrome/browser/sessions/tab_restore_service.cc (right):

http://codereview.chromium.org/92001/diff/19/1006#newcode161
Line 161: window->tabs[entry_index].browser_id = browser->session_id().id();
Perhaps we should move this to PopulateTabFromController.

http://codereview.chromium.org/92001/diff/19/1008
File chrome/test/automation/browser_proxy.cc (right):

http://codereview.chromium.org/92001/diff/19/1008#newcode22
Line 22: bool BrowserProxy::Close() {
This seems like a duplicate of UITest::CloseBrowser. I like that yours is on the
browserproxy better, but yours is missing the WaitForSingleProcess that is in
uitest. Seems like that is needed.

[Pam (message me for reviews), 2009-04-22 18:18:21]

New patch uploaded.

http://codereview.chromium.org/92001/diff/19/1006
File chrome/browser/sessions/tab_restore_service.cc (right):

http://codereview.chromium.org/92001/diff/19/1006#newcode161
Line 161: window->tabs[entry_index].browser_id = browser->session_id().id();
On 2009/04/22 14:45:04, sky wrote:
> Perhaps we should move this to PopulateTabFromController.

I considered that, but it's a bit out of place there, since it has very little
to do with filling the tab's navigation list. But on second thought, I'll just
rename the method more broadly.

Done.

http://codereview.chromium.org/92001/diff/19/1008
File chrome/test/automation/browser_proxy.cc (right):

http://codereview.chromium.org/92001/diff/19/1008#newcode22
Line 22: bool BrowserProxy::Close() {
On 2009/04/22 14:45:04, sky wrote:
> This seems like a duplicate of UITest::CloseBrowser. I like that yours is on
the
> browserproxy better, but yours is missing the WaitForSingleProcess that is in
> uitest. Seems like that is needed.

Ah, I was wondering why I couldn't find a browser Close() already extant. I
expected it to be in BrowserProxy, paralleling TabProxy::Close().

I looked at moving the methods from UITest into BrowserProxy, but the browser
proxy has no easy access to the process that's being closed, so it can't call
WaitForSingleProcess. So I took this out.

[sky, 2009-04-22 18:27:36]

LGTM