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

Issue 12879006: Removed task runners from the DesktopEnviroment interface and introduced ScreenControls/ClientSessio (Closed)

Created:
7 years, 9 months ago by alexeypa (please no reviews)
Modified:
7 years, 9 months ago
Reviewers:
Sergey Ulanov, Wez
CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, simonmorris+watch_chromium.org, hclam+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, rmsousa+watch_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org
Visibility:
Public.

Description

Removed task runners from the DesktopEnviroment interface and introduced ScreenControls/ClientSessionControl interfaces. This CL removes all task runners that used to be passed to methods of DesktopEnviroment and DesktopEnviromentFactory interfaces. Instead each object implementing these interfaces receives the set of task runners it needs in the constructor. This change makes DesktopEnviroment and DesktopEnviromentFactory interfaces cleaner and easier to implement. Added the ScreenControls interface used by the client session to change the screen resolution. Objects implementing ScreenControls are created by DesktopEnvironment::CreateScreenControls() method. DesktopEnviromentFactory::Create() now receives a pointer to the ClientSessionControl interface providing a way to pause, resume, and disconnect the client session. It also receives notifications about the local mouse movements to temporarily block the remote input. The ClientSessionControl interface will be hooked up to the local impit monitor and the host UI once they will be moved to DesktopEnvironment. BUG=104544 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190345

Patch Set 1 #

Patch Set 2 : The input thread should be of TYPE_IO. #

Total comments: 10

Patch Set 3 : Moved the comments from another CL. #

Patch Set 4 : CR feedback. #

Total comments: 18

Patch Set 5 : More CR feedback. #

Patch Set 6 : rebased #

Total comments: 6

Patch Set 7 : rebased #

Patch Set 8 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+624 lines, -566 lines) Patch
M remoting/host/basic_desktop_environment.h View 1 2 3 4 5 6 1 chunk +60 lines, -18 lines 0 comments Download
M remoting/host/basic_desktop_environment.cc View 1 2 3 4 5 6 1 chunk +41 lines, -23 lines 0 comments Download
M remoting/host/chromoting_host.cc View 1 2 3 4 5 6 5 chunks +5 lines, -5 lines 0 comments Download
M remoting/host/chromoting_host_unittest.cc View 1 2 3 4 5 6 2 chunks +7 lines, -19 lines 0 comments Download
M remoting/host/client_session.h View 1 2 3 4 5 6 5 chunks +19 lines, -23 lines 0 comments Download
M remoting/host/client_session.cc View 1 2 3 4 5 6 13 chunks +27 lines, -25 lines 0 comments Download
A remoting/host/client_session_control.h View 1 2 3 4 1 chunk +40 lines, -0 lines 0 comments Download
M remoting/host/client_session_unittest.cc View 1 2 3 4 5 6 4 chunks +11 lines, -27 lines 0 comments Download
M remoting/host/desktop_environment.h View 1 2 3 4 5 6 4 chunks +11 lines, -15 lines 0 comments Download
M remoting/host/desktop_process.h View 2 chunks +4 lines, -0 lines 0 comments Download
M remoting/host/desktop_process.cc View 5 chunks +7 lines, -6 lines 0 comments Download
M remoting/host/desktop_process_main.cc View 2 chunks +16 lines, -2 lines 0 comments Download
M remoting/host/desktop_process_unittest.cc View 1 2 3 4 5 6 4 chunks +8 lines, -21 lines 0 comments Download
M remoting/host/desktop_session_agent.h View 1 2 3 4 5 6 8 chunks +20 lines, -11 lines 0 comments Download
M remoting/host/desktop_session_agent.cc View 1 2 3 4 5 6 10 chunks +46 lines, -27 lines 0 comments Download
M remoting/host/desktop_session_proxy.h View 1 2 3 4 5 6 3 chunks +12 lines, -18 lines 0 comments Download
M remoting/host/desktop_session_proxy.cc View 1 2 3 4 5 6 6 chunks +29 lines, -26 lines 0 comments Download
M remoting/host/host_mock_objects.h View 1 2 3 4 5 6 4 chunks +11 lines, -33 lines 0 comments Download
M remoting/host/host_mock_objects.cc View 1 2 3 4 5 6 3 chunks +10 lines, -22 lines 0 comments Download
M remoting/host/ipc_desktop_environment.h View 1 2 3 4 5 6 5 chunks +19 lines, -17 lines 0 comments Download
M remoting/host/ipc_desktop_environment.cc View 1 2 3 4 5 6 3 chunks +35 lines, -41 lines 0 comments Download
M remoting/host/ipc_desktop_environment_unittest.cc View 1 2 3 4 5 6 9 chunks +53 lines, -29 lines 0 comments Download
A + remoting/host/ipc_screen_controls.h View 1 2 3 2 chunks +8 lines, -8 lines 0 comments Download
A + remoting/host/ipc_screen_controls.cc View 1 2 3 1 chunk +4 lines, -4 lines 0 comments Download
M remoting/host/ipc_session_controller.h View 1 2 3 1 chunk +0 lines, -35 lines 0 comments Download
M remoting/host/ipc_session_controller.cc View 1 2 3 1 chunk +0 lines, -25 lines 0 comments Download
M remoting/host/me2me_desktop_environment.h View 1 2 3 4 2 chunks +12 lines, -8 lines 0 comments Download
M remoting/host/me2me_desktop_environment.cc View 1 2 3 4 5 6 2 chunks +35 lines, -16 lines 0 comments Download
M remoting/host/plugin/host_script_object.cc View 1 chunk +4 lines, -1 line 0 comments Download
M remoting/host/remoting_me2me_host.cc View 1 2 3 4 5 6 1 chunk +9 lines, -1 line 0 comments Download
M remoting/host/resizing_host_observer.h View 1 2 3 2 chunks +5 lines, -5 lines 0 comments Download
A + remoting/host/screen_controls.h View 1 2 3 2 chunks +6 lines, -7 lines 0 comments Download
M remoting/host/session_controller.h View 1 2 3 1 chunk +0 lines, -26 lines 0 comments Download
M remoting/host/win/session_desktop_environment.h View 1 2 3 4 5 6 2 chunks +13 lines, -7 lines 0 comments Download
M remoting/host/win/session_desktop_environment.cc View 1 2 3 4 5 6 1 chunk +33 lines, -12 lines 0 comments Download
M remoting/remoting.gyp View 1 2 3 4 5 6 4 chunks +4 lines, -3 lines 0 comments Download

Messages

Total messages: 24 (0 generated)
alexeypa (please no reviews)
PTAL.
7 years, 9 months ago (2013-03-19 21:23:15 UTC) #1
Wez
On 2013/03/19 21:23:15, alexeypa wrote: > PTAL. sergeyu: PTAL as well.
7 years, 9 months ago (2013-03-20 23:11:07 UTC) #2
Wez
The SessionController & Delegate interfaces don't fit with the current DesktopEnvironment model. DesktopEnvironment is really ...
7 years, 9 months ago (2013-03-20 23:19:23 UTC) #3
alexeypa (please no reviews)
> The SessionController & Delegate interfaces don't fit with the current > DesktopEnvironment model. DesktopEnvironment ...
7 years, 9 months ago (2013-03-21 00:01:48 UTC) #4
alexeypa (please no reviews)
Here is a new version based on our discussion. PTAL.
7 years, 9 months ago (2013-03-21 22:55:22 UTC) #5
Wez
LGTM w/ nits. https://chromiumcodereview.appspot.com/12879006/diff/14001/remoting/host/basic_desktop_environment.h File remoting/host/basic_desktop_environment.h (right): https://chromiumcodereview.appspot.com/12879006/diff/14001/remoting/host/basic_desktop_environment.h#newcode59 remoting/host/basic_desktop_environment.h:59: nit: remove extra blank line https://chromiumcodereview.appspot.com/12879006/diff/14001/remoting/host/client_session.cc ...
7 years, 9 months ago (2013-03-22 01:18:20 UTC) #6
alexeypa (please no reviews)
https://chromiumcodereview.appspot.com/12879006/diff/14001/remoting/host/basic_desktop_environment.h File remoting/host/basic_desktop_environment.h (right): https://chromiumcodereview.appspot.com/12879006/diff/14001/remoting/host/basic_desktop_environment.h#newcode59 remoting/host/basic_desktop_environment.h:59: On 2013/03/22 01:18:21, Wez wrote: > nit: remove extra ...
7 years, 9 months ago (2013-03-22 18:32:41 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alexeypa@chromium.org/12879006/24001
7 years, 9 months ago (2013-03-22 19:51:07 UTC) #8
commit-bot: I haz the power
Failed to trigger a try job on win_rel HTTP Error 400: Bad Request
7 years, 9 months ago (2013-03-22 20:58:26 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alexeypa@chromium.org/12879006/40001
7 years, 9 months ago (2013-03-22 20:59:13 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alexeypa@chromium.org/12879006/40001
7 years, 9 months ago (2013-03-23 15:43:31 UTC) #11
Sergey Ulanov
lgtm with nits https://chromiumcodereview.appspot.com/12879006/diff/40001/remoting/host/basic_desktop_environment.cc File remoting/host/basic_desktop_environment.cc (right): https://chromiumcodereview.appspot.com/12879006/diff/40001/remoting/host/basic_desktop_environment.cc#newcode66 remoting/host/basic_desktop_environment.cc:66: } nit: Add thread DCHECK here? ...
7 years, 9 months ago (2013-03-23 19:50:36 UTC) #12
commit-bot: I haz the power
Failed to apply patch for remoting/host/basic_desktop_environment.cc: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
7 years, 9 months ago (2013-03-24 04:58:39 UTC) #13
alexeypa (please no reviews)
https://chromiumcodereview.appspot.com/12879006/diff/40001/remoting/host/basic_desktop_environment.cc File remoting/host/basic_desktop_environment.cc (right): https://chromiumcodereview.appspot.com/12879006/diff/40001/remoting/host/basic_desktop_environment.cc#newcode66 remoting/host/basic_desktop_environment.cc:66: } On 2013/03/23 19:50:36, sergeyu wrote: > nit: Add ...
7 years, 9 months ago (2013-03-24 06:26:16 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alexeypa@chromium.org/12879006/65001
7 years, 9 months ago (2013-03-24 06:32:23 UTC) #15
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 9 months ago (2013-03-24 06:39:50 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alexeypa@chromium.org/12879006/71001
7 years, 9 months ago (2013-03-24 15:27:00 UTC) #17
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 9 months ago (2013-03-24 15:36:20 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alexeypa@chromium.org/12879006/71001
7 years, 9 months ago (2013-03-24 21:09:59 UTC) #19
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 9 months ago (2013-03-24 21:11:49 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alexeypa@chromium.org/12879006/71001
7 years, 9 months ago (2013-03-24 21:15:23 UTC) #21
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 9 months ago (2013-03-24 21:18:56 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alexeypa@chromium.org/12879006/71001
7 years, 9 months ago (2013-03-25 04:50:59 UTC) #23
commit-bot: I haz the power
7 years, 9 months ago (2013-03-25 06:01:13 UTC) #24
Message was sent while issue was closed.
Change committed as 190345

Powered by Google App Engine
This is Rietveld 408576698