Add a test case to test disabling remote connection.

Add a test case to test disabling remote connection.

BUG=

Committed: https://crrev.com/8a398d63ecd1690e77b4bdcf793e556032be8cd2
Cr-Commit-Position: refs/heads/master@{#312892}

[yihongg1, 2014-12-19 22:31:48]

yihongg@chromium.org changed reviewers:
+ anandc@chromium.org, jamiewalch@chromium.org

[Jamie, 2014-12-19 23:17:26]

LGTM once my comments are addressed.

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/me2me_b...
File chrome/test/remoting/me2me_browsertest.cc (right):

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/me2me_b...
chrome/test/remoting/me2me_browsertest.cc:100:
MANUAL_Me2Me_disable_remote_connection) {
Nit: Disable_Remote_Connection (capitalization).

Also, this file has no other tests in it (I think it's intended to be a base
class for other tests). This test should be in a separate file (see
scrollbar_browsertest.cc for an example).

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:906: "remoting.currentMode
==\"" + mode + "\"");
Nit: The mode should be expressed as one of the remoting.AppMode constants, not
an explicit string, in which case you don't need the quotes.

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:910: ConditionalTimeoutWaiter
hostReadyWaiter(
Nit: hostListReadyWaiter (you're not waiting for the local host to be ready but
the list of hosts).

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:922: this,
"home.host-setup.done"));
Please use remoting.AppMode.HOST_SETUP_DONE instead.

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
File chrome/test/remoting/remote_desktop_browsertest.h (right):

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.h:354: // Test if the remoting
app mode is matching the given mode
s/is matching/matches/

or

s/matching/equal to/

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.h:355: bool
IsAppModeMatching(const std::string& mode);
Similar to the above, IsAppModeEqualTo would be a more grammatically-correct way
of phrasing this (it's also more explicit--"matching" could mean a number of
different things).

[yihongg, 2015-01-16 22:22:31]

The CQ bit was checked by yihongg@google.com

[yihongg, 2015-01-16 22:22:33]

The CQ bit was unchecked by yihongg@google.com

[yihongg1, 2015-01-22 21:55:26]

PTAL

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/me2me_b...
File chrome/test/remoting/me2me_browsertest.cc (right):

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/me2me_b...
chrome/test/remoting/me2me_browsertest.cc:100:
MANUAL_Me2Me_disable_remote_connection) {
On 2014/12/19 23:17:26, Jamie wrote:
> Nit: Disable_Remote_Connection (capitalization).
> 
> Also, this file has no other tests in it (I think it's intended to be a base
> class for other tests). This test should be in a separate file (see
> scrollbar_browsertest.cc for an example).

Done.

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:906: "remoting.currentMode
==\"" + mode + "\"");
On 2014/12/19 23:17:26, Jamie wrote:
> Nit: The mode should be expressed as one of the remoting.AppMode constants,
not
> an explicit string, in which case you don't need the quotes.

Done.

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:910: ConditionalTimeoutWaiter
hostReadyWaiter(
On 2014/12/19 23:17:26, Jamie wrote:
> Nit: hostListReadyWaiter (you're not waiting for the local host to be ready
but
> the list of hosts).

Done.

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:922: this,
"home.host-setup.done"));
On 2014/12/19 23:17:26, Jamie wrote:
> Please use remoting.AppMode.HOST_SETUP_DONE instead.

Done.

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
File chrome/test/remoting/remote_desktop_browsertest.h (right):

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.h:354: // Test if the remoting
app mode is matching the given mode
On 2014/12/19 23:17:26, Jamie wrote:
> s/is matching/matches/
> 
> or
> 
> s/matching/equal to/

Done.

https://codereview.chromium.org/802523004/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.h:355: bool
IsAppModeMatching(const std::string& mode);
On 2014/12/19 23:17:26, Jamie wrote:
> Similar to the above, IsAppModeEqualTo would be a more grammatically-correct
way
> of phrasing this (it's also more explicit--"matching" could mean a number of
> different things).

Done.

[Jamie, 2015-01-23 00:16:14]

LGTM.

Pro-tip: when responding to code-review comments, try not to rebase until you
have an LGTM. It's really useful to be able to look at the diffs relative to the
last time I reviewed the code, because my comments, and your changes in response
to them, are clearly visible. If you include a rebase, then all changes from
that rebase are also included, and I essentially have to go back to reviewing it
as a new CL.

[yihongg1, 2015-01-23 00:18:21]

The CQ bit was checked by yihongg@chromium.org

[commit-bot: I haz the power, 2015-01-23 00:20:40]

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

[commit-bot: I haz the power, 2015-01-23 02:24:36]

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

[commit-bot: I haz the power, 2015-01-23 02:24:36]

Exceeded time limit waiting for builds to trigger.

[yihongg1, 2015-01-23 19:14:28]

The CQ bit was checked by yihongg@chromium.org

[commit-bot: I haz the power, 2015-01-23 19:16:36]

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

[commit-bot: I haz the power, 2015-01-23 19:18:34]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-23 19:19:30]

Patchset 2 (id:??) landed as
https://crrev.com/8a398d63ecd1690e77b4bdcf793e556032be8cd2
Cr-Commit-Position: refs/heads/master@{#312892}