Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(267)

Issue 1097133002: [Webapp Refactor] Remove remoting.SessionConnector. (Closed)

Created:
5 years, 8 months ago by kelvinp
Modified:
5 years, 8 months ago
Reviewers:
Jamie
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org, chromoting-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Webapp Refactor] Remove remoting.SessionConnector. This is a reland of crrev.com/1047413006 remoting.SessionConnector is currently responsible for creating the plugin, the signal strategy and the clientSession. It also magically disposes the plugin when the clientSession finishes. However, the session is not exposed to the caller elsewhere, which makes it hard for the caller to dispose of the session before it is connected, e.g. Cancel a PIN entry. It is currently a stateful object that keeps track of the host, credentials provider, strategy of the created session, which is redundant. This CL 1. Offloads the creation of the clientSession to the remoting.ClientSessionFactory. It also allow the caller to configure a clientSession prior to connecting. The remoting.ClientSessionFactory is essentially stateless except for a few predefined ClientSession construction parameters. 2. Allow the caller to have a different instance of ClientSession.EventHandler for each instance ClientSession created. 3. Revives remoting.MockClientPlugin and uses it for remoting.ClientSessionFactory unittests. Ownership graph before: Activity -> SessionConenctor -> ClientSession Ownership graph after: Activity -> ClientSession TBR=jamiewalch BUG=477522 TEST=All browser tests passed on https://chromium-swarm.appspot.com/user/tasks?sort=created_ts&state=all&limit=10&task_name=chromoting_integration_tests Committed: https://crrev.com/2c5de34072226c91d465349cb249d76d16387bef Cr-Commit-Position: refs/heads/master@{#325887}

Patch Set 1 : Original CL #

Patch Set 2 : Fix merge conflicts #

Unified diffs Side-by-side diffs Delta from patch set Stats (+422 lines, -602 lines) Patch
M remoting/remoting_webapp_files.gypi View 4 chunks +3 lines, -3 lines 0 comments Download
M remoting/webapp/app_remoting/js/app_remoting_activity.js View 4 chunks +15 lines, -9 lines 0 comments Download
M remoting/webapp/base/js/application.js View 1 chunk +0 lines, -2 lines 0 comments Download
M remoting/webapp/base/js/protocol_extension.js View 1 chunk +1 line, -1 line 0 comments Download
M remoting/webapp/crd/js/client_session.js View 7 chunks +88 lines, -10 lines 0 comments Download
A remoting/webapp/crd/js/client_session_factory.js View 1 chunk +134 lines, -0 lines 0 comments Download
A remoting/webapp/crd/js/client_session_factory_unittest.js View 1 chunk +100 lines, -0 lines 0 comments Download
M remoting/webapp/crd/js/desktop_remoting_activity.js View 3 chunks +24 lines, -2 lines 0 comments Download
M remoting/webapp/crd/js/it2me_activity.js View 1 chunk +3 lines, -7 lines 0 comments Download
M remoting/webapp/crd/js/me2me_activity.js View 1 chunk +3 lines, -8 lines 0 comments Download
M remoting/webapp/crd/js/mock_client_plugin.js View 1 7 chunks +49 lines, -43 lines 0 comments Download
D remoting/webapp/crd/js/mock_session_connector.js View 1 chunk +0 lines, -171 lines 0 comments Download
D remoting/webapp/crd/js/session_connector.js View 1 chunk +0 lines, -52 lines 0 comments Download
D remoting/webapp/crd/js/session_connector_impl.js View 1 chunk +0 lines, -291 lines 0 comments Download
M remoting/webapp/files.gni View 1 1 chunk +2 lines, -3 lines 0 comments Download

Messages

Total messages: 6 (3 generated)
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1097133002/40001
5 years, 8 months ago (2015-04-20 17:49:35 UTC) #4
commit-bot: I haz the power
Committed patchset #2 (id:40001)
5 years, 8 months ago (2015-04-20 18:52:11 UTC) #5
commit-bot: I haz the power
5 years, 8 months ago (2015-04-20 18:53:00 UTC) #6
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/2c5de34072226c91d465349cb249d76d16387bef
Cr-Commit-Position: refs/heads/master@{#325887}

Powered by Google App Engine
This is Rietveld 408576698