[Chromoting] Move app-specific code out of remoting.js

[Chromoting] Move app-specific code out of remoting.js

After this cl, the last remaining reference to remoting.AppMode in AppRemoting is in app_remoting.js. That will be addressed in a followup cl.

Move the following out of remoting.js and into app-specific locations:
* remoting.signOut (into crd_event_handlers.js)
* remoting.showErrorMessage (into desktop_remoting.js)
* isWindowed_ (into desktop_remoting.js)
* remoting.promptClose (into desktop_remoting.js)
* remoting.isMe2MeInstallable (into local_host_section.js)

Rename Delegate.signInFailed -> Delegate.handleAuthError
Add Application.onAuthError to call Delegate.handleAuthError

Change references to global remoting.showErrorMessage to remoting.app.onAuthError

BUG=

Committed: https://crrev.com/a59c430f5028228e62f780b2e129e93a4cb72843
Cr-Commit-Position: refs/heads/master@{#321502}

[garykac, 2015-03-19 01:03:44]

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/crd_...
File remoting/webapp/crd/js/crd_event_handlers.js (right):

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/crd_...
remoting/webapp/crd/js/crd_event_handlers.js:95: remoting.signOut = function() {
moved here from remoting.js. It is only used in this file.

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/desk...
File remoting/webapp/crd/js/desktop_remoting.js (right):

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/desk...
remoting/webapp/crd/js/desktop_remoting.js:324:
remoting.DesktopRemoting.prototype.handleAuthError = function(error) {
function copied from remoting.showErrorMessage

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/desk...
remoting/webapp/crd/js/desktop_remoting.js:350:
remoting.DesktopRemoting.prototype.isWindowed_ = function(callback) {
moved here from remoting.js

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/desk...
remoting/webapp/crd/js/desktop_remoting.js:376:
remoting.DesktopRemoting.prototype.promptClose_ = function() {
moved here from remoting.js

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/loca...
File remoting/webapp/crd/js/local_host_section.js (right):

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/loca...
remoting/webapp/crd/js/local_host_section.js:118: remoting.isMe2MeInstallable =
function() {
moved here from remoting.js

[garykac, 2015-03-19 01:03:53]

garykac@chromium.org changed reviewers:
+ kelvinp@chromium.org

[garykac, 2015-03-19 01:04:24]

Please see comments in Patchset #1

[kelvinp, 2015-03-19 17:50:20]

Thank you for the cleanup.  I like the general idea.
The only outstanding feedback is that we are overloading handleAuthError
especially on desktopRemoting to handle non-auth error, which can be misleading
at times.  I would suggest having a deskopRemoting declare a static method,
remoting.DesktopRemoting.showErrorMessage() in the time being.

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/crd_...
File remoting/webapp/crd/js/crd_event_handlers.js (right):

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/crd_...
remoting/webapp/crd/js/crd_event_handlers.js:95: remoting.signOut = function() {
On 2015/03/19 01:03:43, garykac wrote:
> moved here from remoting.js. It is only used in this file.

Acknowledged.

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/crd_...
File remoting/webapp/crd/js/crd_main.js (right):

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/crd_...
remoting/webapp/crd/js/crd_main.js:21:
remoting.app.onAuthError.bind(remoting.app));
This is a bit misleading as the error that the host list reports are not auth
issues but errors in deleting/renaming/unregistering the host.

What about keeping remoting.showErrorMessage() but moving it to
crd_events_handlers.js or crd_main.js?

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/desk...
File remoting/webapp/crd/js/desktop_remoting.js (right):

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/desk...
remoting/webapp/crd/js/desktop_remoting.js:377: if
(remoting.desktopConnectedView &&
desktopConnectedView is gone now.  Need remerging.
You can get the mode from getSessionConnector().

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/loca...
File remoting/webapp/crd/js/local_host_section.js (right):

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/loca...
remoting/webapp/crd/js/local_host_section.js:118: remoting.isMe2MeInstallable =
function() {
On 2015/03/19 01:03:44, garykac wrote:
> moved here from remoting.js

It looks like the function is only used here.
Please make it either an anonymous function or a private member of
LocalHostSecion.

[garykac, 2015-03-19 21:58:38]

ptal

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/crd_...
File remoting/webapp/crd/js/crd_main.js (right):

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/crd_...
remoting/webapp/crd/js/crd_main.js:21:
remoting.app.onAuthError.bind(remoting.app));
On 2015/03/19 17:50:20, kelvinp wrote:
> This is a bit misleading as the error that the host list reports are not auth
> issues but errors in deleting/renaming/unregistering the host.
> 
> What about keeping remoting.showErrorMessage() but moving it to
> crd_events_handlers.js or crd_main.js?

Done.

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/desk...
File remoting/webapp/crd/js/desktop_remoting.js (right):

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/desk...
remoting/webapp/crd/js/desktop_remoting.js:377: if
(remoting.desktopConnectedView &&
On 2015/03/19 17:50:20, kelvinp wrote:
> desktopConnectedView is gone now.  Need remerging.
> You can get the mode from getSessionConnector().

Acknowledged.

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/loca...
File remoting/webapp/crd/js/local_host_section.js (right):

https://codereview.chromium.org/1020743002/diff/1/remoting/webapp/crd/js/loca...
remoting/webapp/crd/js/local_host_section.js:118: remoting.isMe2MeInstallable =
function() {
On 2015/03/19 17:50:20, kelvinp wrote:
> On 2015/03/19 01:03:44, garykac wrote:
> > moved here from remoting.js
> 
> It looks like the function is only used here.
> Please make it either an anonymous function or a private member of
> LocalHostSecion.

Done.

[kelvinp, 2015-03-19 23:25:20]

lgtm with Nits

https://codereview.chromium.org/1020743002/diff/60001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/local_host_section.js (right):

https://codereview.chromium.org/1020743002/diff/60001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/local_host_section.js:116: * @return {boolean}
Make private please.

[garykac, 2015-03-19 23:49:06]

https://codereview.chromium.org/1020743002/diff/60001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/local_host_section.js (right):

https://codereview.chromium.org/1020743002/diff/60001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/local_host_section.js:116: * @return {boolean}
On 2015/03/19 23:25:20, kelvinp wrote:
> Make private please.

Done.

[garykac, 2015-03-19 23:49:22]

The CQ bit was checked by garykac@chromium.org

[garykac, 2015-03-19 23:49:22]

The patchset sent to the CQ was uploaded after l-g-t-m from kelvinp@chromium.org
Link to the patchset: https://codereview.chromium.org/1020743002/#ps80001
(title: "Make isMe2MeInstallable private")

[commit-bot: I haz the power, 2015-03-19 23:49:56]

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

[commit-bot: I haz the power, 2015-03-20 02:31:51]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-03-20 02:32:34]

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