Fixes bug where triggering session restore while the browser was...

Fixes bug where triggering session restore while the browser was
already running would end up creating an extra tab.

BUG=11594
TEST=open chrome with a single tabbed browser, turn on session
  restore, navigate to a page with a popup, close the tabbed browser,
  create a new window ala control-n (or double click on the desktop),
  and make the restored window doesn't end upw 


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

[brettw, 2010-03-25 23:03:37]

http://codereview.chromium.org/1371002/diff/11001/12008
File chrome/browser/browser_init.cc (right):

http://codereview.chromium.org/1371002/diff/11001/12008#newcode509
chrome/browser/browser_init.cc:509: if ((process_startup &&
!OpenStartupURLs(urls_to_open)) ||
My brain is not large enough to parse this if statement. Can you try to make it
more clear with a helper function or something. Maybe if it had a bunch of
conditions and early returns with comments it would be super easy to follow.

[sky, 2010-03-25 23:45:04]

http://codereview.chromium.org/1371002/diff/11001/12008
File chrome/browser/browser_init.cc (right):

http://codereview.chromium.org/1371002/diff/11001/12008#newcode509
chrome/browser/browser_init.cc:509: if ((process_startup &&
!OpenStartupURLs(urls_to_open)) ||
On 2010/03/25 23:03:37, brettw wrote:
> My brain is not large enough to parse this if statement. Can you try to make
it
> more clear with a helper function or something. Maybe if it had a bunch of
> conditions and early returns with comments it would be super easy to follow.

Done.
New patch uploaded.

[brettw, 2010-03-26 00:33:32]

This is a million times better, LGTM

http://codereview.chromium.org/1371002/diff/19001/20008
File chrome/browser/browser_init.cc (right):

http://codereview.chromium.org/1371002/diff/19001/20008#newcode634
chrome/browser/browser_init.cc:634: // at various point, such as if there is
only an app window running and the
grammar: point -> points

[Paweł Hajdan Jr., 2010-03-26 08:03:20]

Drive-by with minor test comments.

http://codereview.chromium.org/1371002/diff/19001/20007
File chrome/browser/sessions/session_restore_browsertest.cc (right):

http://codereview.chromium.org/1371002/diff/19001/20007#newcode42
chrome/browser/sessions/session_restore_browsertest.cc:42:
EXPECT_TRUE(new_browser != NULL);
This should be ASSERT_TRUE to avoid a crash.

http://codereview.chromium.org/1371002/diff/19001/20007#newcode45
chrome/browser/sessions/session_restore_browsertest.cc:45: EXPECT_EQ(1,
new_browser->tab_count());
Same here.