Add support for deferring app initialization when testing.

Add support for deferring app initialization when testing.

This will allow certain app features such as authentication and loading of the
host list to be replaced with mocks before they are used; in normal operation,
these features are run automatically when the app is loaded, giving browser
tests no opportunity to intervene.

This is achieved by using the "source" URL parameter. Production code is
unaffected by virtue of the fact that "source" is not passed by default, and
even if a user is running with a command-line override, it will never be set
to "test". As protection against the possibility that the previous sentence
is false, when started in this test mode, initialization proceeds via a
button click, making it user-accessible; this CL also adds a test that this
button works.

Committed: https://crrev.com/0932e48fc8387c01cd1edf092c42957f794b9b01
Cr-Commit-Position: refs/heads/master@{#308513}

[Jamie, 2014-12-02 01:30:12]

jamiewalch@chromium.org changed reviewers:
+ cylee@chromium.org, garykac@chromium.org, weitaosu@chromium.org

[Jamie, 2014-12-02 01:30:12]

jamiewalch@chromium.org changed required reviewers:
+ weitaosu@chromium.org

[Jamie, 2014-12-02 01:30:13]

Weitao: PTAL

Gary: FYI, since I'm modifying some of the start-up code you've been looking at.
I'll probably need to rebase this before landing it.

cylee@: I'd like your input on the question of whether or not it makes sense to
add a BROWSER_TEST AppLaunchSource for this use-case.

https://codereview.chromium.org/771003002/diff/20001/chrome/test/remoting/pag...
File chrome/test/remoting/page_load_notification_observer.h (right):

https://codereview.chromium.org/771003002/diff/20001/chrome/test/remoting/pag...
chrome/test/remoting/page_load_notification_observer.h:26: void
set_ignore_url_parameters(bool ignore_url_parameters) {
This is needed because the URL is different depending on whether the app is
started in test mode or not. But I think it's a more robust design anyway.

https://codereview.chromium.org/771003002/diff/20001/chrome/test/remoting/rem...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):

https://codereview.chromium.org/771003002/diff/20001/chrome/test/remoting/rem...
chrome/test/remoting/remote_desktop_browsertest.cc:186: // even if they have the
trace_app_source command-line option enabled.
cylee@: What I would like to do here is add a BROWSER_TEST member to the
AppLaunchSource enum, but the comment about its inclusion in UMA stats makes me
suspect that I don't fully understand the ramifications of doing so. Would you
be supportive of a separate CL to add that so that I can use it in this one? If
not, then I think this is safe, but it's a bit of a hack.

[cylee1, 2014-12-02 14:10:33]

https://codereview.chromium.org/771003002/diff/20001/chrome/test/remoting/rem...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):

https://codereview.chromium.org/771003002/diff/20001/chrome/test/remoting/rem...
chrome/test/remoting/remote_desktop_browsertest.cc:186: // even if they have the
trace_app_source command-line option enabled.
On 2014/12/02 01:30:13, Jamie wrote:
> cylee@: What I would like to do here is add a BROWSER_TEST member to the
> AppLaunchSource enum, but the comment about its inclusion in UMA stats makes
me
> suspect that I don't fully understand the ramifications of doing so. Would you
> be supportive of a separate CL to add that so that I can use it in this one?
If
> not, then I think this is safe, but it's a bit of a hack.

Adding a new enum BROWSER_TEST sounds much reasonable than using
SOURCE_ABOUT_PAGE to me.
It would be recorded in ChromeOS UMA system but I guess it's fine.

Note that you'll need to modify 
extensions/common/api/app_runtime.idl
tools/metrics/histograms/histograms.xml
extensions/common/constants.h
and 
extensions/browser/api/app_runtime/app_runtime_api.cc
for the enum mapping.

[garykac, 2014-12-02 16:43:22]

https://codereview.chromium.org/771003002/diff/20001/remoting/webapp/crd/js/e...
File remoting/webapp/crd/js/event_handlers.js (right):

https://codereview.chromium.org/771003002/diff/20001/remoting/webapp/crd/js/e...
remoting/webapp/crd/js/event_handlers.js:118: remoting.initForTesting();
Note that https://codereview.chromium.org/742473002/ removes the remoting.init()
call here.

crd_main.js will need to be updated instead.

https://codereview.chromium.org/771003002/diff/20001/remoting/webapp/crd/js/r...
File remoting/webapp/crd/js/remoting.js (right):

https://codereview.chromium.org/771003002/diff/20001/remoting/webapp/crd/js/r...
remoting/webapp/crd/js/remoting.js:58: remoting.initForTesting = function() {
This belongs in (the newly created) crd_main.js.

[Jamie, 2014-12-12 21:35:27]

jamiewalch@chromium.org changed reviewers:
- cylee@chromium.org

[Jamie, 2014-12-12 21:35:27]

ptal

[weitao, 2014-12-12 22:49:52]

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
chrome/test/remoting/remote_desktop_browsertest.cc:186: }
Where is the "test" URL parameter added?

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
File chrome/test/remoting/remote_desktop_browsertest.h (right):

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
chrome/test/remoting/remote_desktop_browsertest.h:234: return
GURL("chrome-extension://" + ChromotingID() + "/main.html");
Does the V2 app no longer include isKioskSession in its url? Or does this
correspond to the change in GetCurrentURL() below?

[Jamie, 2014-12-13 01:03:43]

ptal

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
chrome/test/remoting/remote_desktop_browsertest.cc:186: }
On 2014/12/12 22:49:52, weitaosu wrote:
> Where is the "test" URL parameter added?

extensions::FeatureSwitch::trace_app_source() is a command-line option that
causes apps to be launched with a |source| URL parameter. The source is
specified in AppLaunchParams; specifically, in this case it's specified on line
193. If |defer_start| is true, we force that parameter to true.

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
File chrome/test/remoting/remote_desktop_browsertest.h (right):

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
chrome/test/remoting/remote_desktop_browsertest.h:234: return
GURL("chrome-extension://" + ChromotingID() + "/main.html");
On 2014/12/12 22:49:52, weitaosu wrote:
> Does the V2 app no longer include isKioskSession in its url? Or does this
> correspond to the change in GetCurrentURL() below?

Yes it does, but I've modified the URL matcher to ignore parameters (since the
parameters depend on whether the app is being started in deferred mode or not).

[Jamie, 2014-12-15 19:12:23]

Ping Weitao.

[weitao, 2014-12-16 01:06:27]

LGTM with nits.

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
chrome/test/remoting/remote_desktop_browsertest.cc:186: }
On 2014/12/13 01:03:43, Jamie wrote:
> On 2014/12/12 22:49:52, weitaosu wrote:
> > Where is the "test" URL parameter added?
> 
> extensions::FeatureSwitch::trace_app_source() is a command-line option that
> causes apps to be launched with a |source| URL parameter. The source is
> specified in AppLaunchParams; specifically, in this case it's specified on
line
> 193. If |defer_start| is true, we force that parameter to true.

I see. The combination of trace_app_source being true and SOURCE_TEST passed to
AppLaunchParams will add "source='test'" to the URL. It took me a moment to
understand that. Could you please add a short comment explaining that?

[Jamie, 2014-12-16 01:33:51]

FYI

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):

https://codereview.chromium.org/771003002/diff/60001/chrome/test/remoting/rem...
chrome/test/remoting/remote_desktop_browsertest.cc:186: }
On 2014/12/16 01:06:27, weitaosu wrote:
> On 2014/12/13 01:03:43, Jamie wrote:
> > On 2014/12/12 22:49:52, weitaosu wrote:
> > > Where is the "test" URL parameter added?
> > 
> > extensions::FeatureSwitch::trace_app_source() is a command-line option that
> > causes apps to be launched with a |source| URL parameter. The source is
> > specified in AppLaunchParams; specifically, in this case it's specified on
> line
> > 193. If |defer_start| is true, we force that parameter to true.
> 
> I see. The combination of trace_app_source being true and SOURCE_TEST passed
to
> AppLaunchParams will add "source='test'" to the URL. It took me a moment to
> understand that. Could you please add a short comment explaining that?

Done.

[Jamie, 2014-12-16 01:33:55]

The CQ bit was checked by jamiewalch@chromium.org

[commit-bot: I haz the power, 2014-12-16 01:34:52]

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

[commit-bot: I haz the power, 2014-12-16 02:50:06]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2014-12-16 02:50:56]

Patchset 6 (id:??) landed as
https://crrev.com/0932e48fc8387c01cd1edf092c42957f794b9b01
Cr-Commit-Position: refs/heads/master@{#308513}