Automate more Better Session Restore tests: Crashes.

Automate more Better Session Restore tests: Crashes.

BUG=153260
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173732

[marja, 2012-12-17 16:15:51]

ptal

[jochen (gone - plz use gerrit), 2012-12-17 16:21:20]

lgtm with nits

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
File chrome/browser/sessions/base_session_service.h (right):

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/base_session_service.h:166: friend class
BetterSessionRestoreCrashTest;
empty line after this

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
File chrome/browser/sessions/better_session_restore_browsertest.cc (right):

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:259: std::string
fake_server_address_;
since you're only reading these values, it would be cleaner to add getters
instead of exposing member variables

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:516: virtual void
SetUpOnMainThread() OVERRIDE {
you should invoke the BetterSessionRestoreTest::SetUpOnMainThread() here as well

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:519: }
nit empty line after this

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:528: Browser*
browser1 = FindOneOtherBrowserForProfile(profile, NULL);
can you pick a more descriptive name?

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:552: Browser*
browser2 = FindOneOtherBrowserForProfile(profile, browser1);
same here

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:576: };
DISALLOW_COPY_AND_ASSIGN

[marja, 2012-12-17 16:45:58]

thx

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
File chrome/browser/sessions/base_session_service.h (right):

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/base_session_service.h:166: friend class
BetterSessionRestoreCrashTest;
On 2012/12/17 16:21:20, jochen wrote:
> empty line after this

Done.

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
File chrome/browser/sessions/better_session_restore_browsertest.cc (right):

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:259: std::string
fake_server_address_;
On 2012/12/17 16:21:20, jochen wrote:
> since you're only reading these values, it would be cleaner to add getters
> instead of exposing member variables

Done.

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:516: virtual void
SetUpOnMainThread() OVERRIDE {
On 2012/12/17 16:21:20, jochen wrote:
> you should invoke the BetterSessionRestoreTest::SetUpOnMainThread() here as
well

Done.

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:519: }
On 2012/12/17 16:21:20, jochen wrote:
> nit empty line after this

Done.

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:528: Browser*
browser1 = FindOneOtherBrowserForProfile(profile, NULL);
On 2012/12/17 16:21:20, jochen wrote:
> can you pick a more descriptive name?

Done.

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:552: Browser*
browser2 = FindOneOtherBrowserForProfile(profile, browser1);
On 2012/12/17 16:21:20, jochen wrote:
> same here

Done.

https://chromiumcodereview.appspot.com/11597010/diff/1/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:576: };
On 2012/12/17 16:21:20, jochen wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

[commit-bot: I haz the power, 2012-12-17 16:46:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/11597010/7001

[commit-bot: I haz the power, 2012-12-17 19:21:08]

Retried try job too often on linux_chromeos for step(s) browser_tests

[commit-bot: I haz the power, 2012-12-18 10:14:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/11597010/7001

[commit-bot: I haz the power, 2012-12-18 11:33:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/11597010/18002

[marja, 2012-12-18 12:24:55]

i needed to fix this to make it work on chromeos, jochen@, can you review the
additions to startup_browser_creator.* ?

[jochen (gone - plz use gerrit), 2012-12-18 12:26:55]

lgtm

[commit-bot: I haz the power, 2012-12-18 12:27:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/11597010/1005

[commit-bot: I haz the power, 2012-12-18 15:19:52]

Change committed as 173732