[Webapp Refactor] Implements remoting.Activity.

[Webapp Refactor] Implements remoting.Activity.

This CL abstracts Me2Me and It2Me under one common interface
remoting.Activity.

Instead of littering desktop_remoting.js with if statements based on the
mode, all Me2Me specific logic can be moved into
remoting.Me2MeActivity and likewise for It2Me.

BUG=466406
NOTRY=true
TEST=All integration tests passed on chromoting bots.
https://chromium-swarm.appspot.com/user/task/26aed85d041d9d10

Committed: https://crrev.com/3b3e16c4848a6c43874570815fb283e8c653a17f
Cr-Commit-Position: refs/heads/master@{#324156}

[kelvinp, 2015-04-02 23:18:10]

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

[kelvinp, 2015-04-02 23:27:56]

Patchset #1 (id:1) has been deleted

[kelvinp, 2015-04-02 23:28:10]

PTAL

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/base/js...
File remoting/webapp/base/js/input_dialog.js (right):

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/input_dialog.js:57:
remoting.InputDialog.prototype.inputField = function() {
Used by It2MeConnectFlow to clear the inputField upon a succesful connection.

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/htm...
File remoting/webapp/crd/html/dialog_client_session_finished.html (right):

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/htm...
remoting/webapp/crd/html/dialog_client_session_finished.html:23:
i18n-content="RECONNECT">
In today's code, we always change the label to RECONNECT upon a successful
connection, and we never show this UI if the connection failed.  I simplified
the code by always defaulting it to RECONNECT and same for the other changes in
this file.

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/client_session.js (left):

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/client_session.js:553:
remoting.ClientSession.prototype.requestPairing = function(clientName, onDone) {
No longer needed and pairing request will be routed to the plugin directly

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_remoting.js (left):

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:160: var button1 =
document.getElementById('client-reconnect-button');
This code is removed altogether as we always change the label to RECONNECT upon
a successful connection, and we never show this UI if the connection failed.

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:168: this.refreshHostJidIfOffline_ =
true;
Moved to Me2MeConnectFlow.onConnected();

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:170:
document.getElementById('access-code-entry').value = '';
Moved to It2MeConnectFlow.onConnected();

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:186: if
(remoting.app.getConnectionMode() === remoting.Application.Mode.ME2ME) {
Moved to Me2MeConnectFlow.onConnected();

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:201: if (remoting.pairingRequested) {
Moved to PinDialog.requestPairingIfNecessary();

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:254: var that = this;
Moved to Me2MeConnectFlow.onConnectionFailed();

[Jamie, 2015-04-03 19:54:24]

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/base/js...
File remoting/webapp/base/js/input_dialog.js (right):

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/input_dialog.js:57:
remoting.InputDialog.prototype.inputField = function() {
On 2015/04/02 23:28:09, kelvinp wrote:
> Used by It2MeConnectFlow to clear the inputField upon a succesful connection.

If it would make the code cleaner, I would be okay with clearing the field when
the dialog is dismissed. IIRC, we didn't blank it on an unsuccessful attempt in
case it's only wrong by one digit, in which case it doesn't need to be retyped
in its entirety, but I don't think that's a significant usability concern.

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

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/connect_flow.js:30:
remoting.ConnectFlow.prototype.onConnectionFailed = function(error) {};
This interface is a bit confusing. I expected to see a start() method, perhaps
returning a Promise to indicate success/failure. The start() method clearly
belongs on the interface, because it's how a client of this API uses it, but the
other methods seem more like callbacks--they're how an implementation of this
interface notifies its client of various events.

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

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:35: this.connectedView_ = null;
Dispose of flow_ as well?

[kelvinp, 2015-04-06 23:01:39]

PTAL

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

https://codereview.chromium.org/1054273002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:35: this.connectedView_ = null;
On 2015/04/03 19:54:23, Jamie wrote:
> Dispose of flow_ as well?

The Me2MeActivity triggers a reconnect underneath the hood on connection failed,
but it won't re-notify the DesktopRemoting upon successful re-connection.
If we dispose the flow here, the flow won't be recreated.
The underlying problem will go away once we make the Activity (previously known
as flow) to own the client session.

[Jamie, 2015-04-07 21:20:45]

LGTM, but please update the CL description since I think it's out of date now.

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/activity.js (right):

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/activity.js:16: * 2. Handling connection failure and
retry if necessary.
s/failure/failures/
s/retry/retrying/

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/activity.js:23: remoting.Activity = function() {}
Nit: Terminating semicolon.

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_remoting.js (right):

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:34:
base.dispose(this.connectedView_);
Please add a comment explaining that the activity is dispose()d in the
connection methods (and why).

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/it2me_connect_flow.js (right):

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/it2me_connect_flow.js:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
Rename this file to match the class it defines.

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/me2me_connect_flow.js (right):

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/me2me_connect_flow.js:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
Rename to match class definition.

[kelvinp, 2015-04-07 22:48:19]

FYI

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/activity.js (right):

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/activity.js:16: * 2. Handling connection failure and
retry if necessary.
On 2015/04/07 21:20:44, Jamie wrote:
> s/failure/failures/
> s/retry/retrying/

Done.

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/activity.js:23: remoting.Activity = function() {}
On 2015/04/07 21:20:44, Jamie wrote:
> Nit: Terminating semicolon.

Done.

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_remoting.js (right):

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:34:
base.dispose(this.connectedView_);
On 2015/04/07 21:20:44, Jamie wrote:
> Please add a comment explaining that the activity is dispose()d in the
> connection methods (and why).

Done.

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/it2me_connect_flow.js (right):

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/it2me_connect_flow.js:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
On 2015/04/07 21:20:44, Jamie wrote:
> Rename this file to match the class it defines.

Done.

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/me2me_connect_flow.js (right):

https://codereview.chromium.org/1054273002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/me2me_connect_flow.js:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
On 2015/04/07 21:20:44, Jamie wrote:
> Rename to match class definition.

Done.

[kelvinp, 2015-04-07 22:48:23]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2015-04-07 22:48:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1054273002/#ps60001
(title: "Reviewer's feedback")

[commit-bot: I haz the power, 2015-04-07 22:50:03]

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

[kelvinp, 2015-04-07 23:43:21]

The CQ bit was unchecked by kelvinp@chromium.org

[kelvinp, 2015-04-07 23:48:07]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2015-04-07 23:48:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1054273002/#ps80001
(title: "Fix typo")

[commit-bot: I haz the power, 2015-04-07 23:49:20]

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

[commit-bot: I haz the power, 2015-04-07 23:50:40]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-04-07 23:51:27]

Patchset 4 (id:??) landed as
https://crrev.com/3b3e16c4848a6c43874570815fb283e8c653a17f
Cr-Commit-Position: refs/heads/master@{#324156}