Browser test for launching guest mode from the User Manager.

Browser test for launching guest mode from the User Manager.

Also test closing a guest browser and ensuring the cookies get cleared.

BUG=360118, 450903, 445036
Committed: https://crrev.com/f37f4444c07e2ed5275e6c939ddae807f3b51a94
Cr-Commit-Position: refs/heads/master@{#318084}

[Mike Lerman, 2015-01-23 17:59:27]

mlerman@chromium.org changed reviewers:
+ noms@chromium.org

[Mike Lerman, 2015-01-23 17:59:27]

Hey noms,

I wrote a test for Guest. Should help the bots catch things like 450903. Thanks!

[noms (inactive), 2015-01-29 16:17:06]

woohoo! lgtm % nit

https://codereview.chromium.org/871893003/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/user_manager_ui_browsertest.cc (right):

https://codereview.chromium.org/871893003/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/user_manager_ui_browsertest.cc:28: int
CountBrowsersForProfileByPath(const base::FilePath& profile_path) {
nit: wrong indentation (don't indent inside the namespace)

[noms (inactive), 2015-01-29 16:17:45]

hmm can you run some bots, though? on your first patch, the test seem to fail on
windows :(

[Mike Lerman, 2015-02-20 16:10:24]

Hey Monica -

I made more tests! Woo! PTAL :)

https://codereview.chromium.org/871893003/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/user_manager_ui_browsertest.cc (right):

https://codereview.chromium.org/871893003/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/user_manager_ui_browsertest.cc:28: int
CountBrowsersForProfileByPath(const base::FilePath& profile_path) {
On 2015/01/29 16:17:06, Monica Dinculescu wrote:
> nit: wrong indentation (don't indent inside the namespace)

Done.

[Mike Lerman, 2015-02-20 16:44:53]

mlerman@chromium.org changed reviewers:
+ pkasting@chromium.org

[Mike Lerman, 2015-02-20 16:44:55]

Hi Peter,

I had duplicated an include in a test file I submitted in a recent CL. I'm
removing it here - PTAL at c/b/ui/browser_command_controller_browsertest.cc.
Thanks!

[Peter Kasting, 2015-02-20 20:34:55]

On 2015/02/20 16:44:55, Mike Lerman wrote:
> Hi Peter,
> 
> I had duplicated an include in a test file I submitted in a recent CL. I'm
> removing it here - PTAL at c/b/ui/browser_command_controller_browsertest.cc.
> Thanks!

LGTM -- feel free to TBR OWNERS for that sort of a trivial mechanical change if
it makes your life easier.

[Mike Lerman, 2015-02-23 14:48:53]

New patchsets have been uploaded after l-g-t-m from pkasting@chromium.org

[Mike Lerman, 2015-02-23 17:46:44]

Hi Monica,

Any thoughts on the new fanciness in this CL?

[noms (inactive), 2015-02-23 17:57:56]

looks good. just a couple of comments

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
File chrome/browser/profiles/profile_window_browsertest.cc (right):

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:102: // listen for the
notification.
I would remove this MessaheLoop wait, maybe, if you're already waiting for the
observer. The comment makes me want to know why it's being racy, so it's a bit
confusing overall.

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:112: EXPECT_TRUE(browser);
Should this be an ASSERT_TRUE instead?

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:132: OpenGuestBrowser();
maybe add an expect_true that this isn't null? this test looks lonely :)

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:147:
base::FilePath(base::FilePath::kCurrentDirectory),
nit: too much indent

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:165: // Before navigation
there are no cookies at the URL.
nit: s/at the/for the maybe? (here and below)

[Mike Lerman, 2015-02-23 18:09:42]

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
File chrome/browser/profiles/profile_window_browsertest.cc (right):

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:102: // listen for the
notification.
On 2015/02/23 17:57:56, Monica Dinculescu wrote:
> I would remove this MessaheLoop wait, maybe, if you're already waiting for the
> observer. The comment makes me want to know why it's being racy, so it's a bit
> confusing overall.

Done.

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:112: EXPECT_TRUE(browser);
On 2015/02/23 17:57:56, Monica Dinculescu wrote:
> Should this be an ASSERT_TRUE instead?

You can't ASSERT in a method that returns something.

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:132: OpenGuestBrowser();
On 2015/02/23 17:57:56, Monica Dinculescu wrote:
> maybe add an expect_true that this isn't null? this test looks lonely :)

Done.

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:147:
base::FilePath(base::FilePath::kCurrentDirectory),
On 2015/02/23 17:57:56, Monica Dinculescu wrote:
> nit: too much indent

Bye bye Indent.

https://codereview.chromium.org/871893003/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:165: // Before navigation
there are no cookies at the URL.
On 2015/02/23 17:57:56, Monica Dinculescu wrote:
> nit: s/at the/for the maybe? (here and below)

Done.

[Mike Lerman, 2015-02-23 18:39:09]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2015-02-23 18:39:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/871893003/140001

[Mike Lerman, 2015-02-23 20:45:53]

The CQ bit was checked by mlerman@chromium.org

[Mike Lerman, 2015-02-23 20:45:54]

The patchset sent to the CQ was uploaded after l-g-t-m from noms@chromium.org,
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/871893003/#ps160001
(title: "ChromeOS, ChromeOS, why for art thou ChromeOS?")

[commit-bot: I haz the power, 2015-02-23 20:46:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/871893003/160001

[commit-bot: I haz the power, 2015-02-24 01:12:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-24 01:12:16]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mike Lerman, 2015-02-25 17:01:19]

The CQ bit was checked by mlerman@chromium.org

[Mike Lerman, 2015-02-25 17:01:20]

The patchset sent to the CQ was uploaded after l-g-t-m from noms@chromium.org,
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/871893003/#ps200001
(title: "Test desktop only")

[commit-bot: I haz the power, 2015-02-25 17:01:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/871893003/200001

[commit-bot: I haz the power, 2015-02-25 18:51:22]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2015-02-25 18:52:18]

Patchset 11 (id:??) landed as
https://crrev.com/f37f4444c07e2ed5275e6c939ddae807f3b51a94
Cr-Commit-Position: refs/heads/master@{#318084}