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

Issue 11316247: Use AutoThread in ChromotingHostContext & NPAPI plugin. (Closed)

Created:
8 years ago by Wez
Modified:
8 years ago
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, Timur Iskhodzhanov
Visibility:
Public.

Description

Use AutoThread in ChromotingHostContext & NPAPI plugin. Callers now create ChromotingHostContext to create a set of threads for host tasks to run on, and pass the threads' TaskRunners to each host component explicitly. The ChromotingHostContext can then be torn down as soon as the caller no longer needs to create new components using the threads, and the threads themselves will exit only when the created components no longer require them. This is a re-land of 11094056, which failed on the valgrind & Windows TSan bots due to ChromotingHostContext.StartAndStop not running its message loop to allow the context's threads join tasks to execute. TBR=alexeypa Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170360

Patch Set 1 #

Patch Set 2 : Fix ChromotingHostContext.StartAndStop test to run message loop. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+137 lines, -191 lines) Patch
M remoting/host/chromoting_host_context.h View 2 chunks +30 lines, -45 lines 0 comments Download
M remoting/host/chromoting_host_context.cc View 1 chunk +60 lines, -66 lines 0 comments Download
M remoting/host/chromoting_host_context_unittest.cc View 1 2 chunks +19 lines, -11 lines 0 comments Download
M remoting/host/chromoting_host_unittest.cc View 5 chunks +13 lines, -28 lines 0 comments Download
M remoting/host/desktop_environment_factory.cc View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/host/host_mock_objects.h View 1 chunk +0 lines, -17 lines 0 comments Download
M remoting/host/host_mock_objects.cc View 1 chunk +0 lines, -7 lines 0 comments Download
M remoting/host/plugin/host_script_object.h View 1 chunk +1 line, -1 line 0 comments Download
M remoting/host/plugin/host_script_object.cc View 5 chunks +8 lines, -10 lines 0 comments Download
M remoting/host/remoting_me2me_host.cc View 1 chunk +4 lines, -6 lines 0 comments Download
M remoting/host/win/session_desktop_environment_factory.cc View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Wez
FYI, this patch gives no errors under Windows TSan on my local box (I notice ...
8 years ago (2012-11-29 23:33:09 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wez@chromium.org/11316247/2001
8 years ago (2012-11-29 23:33:40 UTC) #2
commit-bot: I haz the power
8 years ago (2012-11-30 02:16:22 UTC) #3
Message was sent while issue was closed.
Change committed as 170360

Powered by Google App Engine
This is Rietveld 408576698