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

Issue 133073005: Trusted plugin: Break up InitCommunication(). (Closed)

Created:
6 years, 11 months ago by teravest
Modified:
6 years, 11 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Trusted plugin: Break up InitCommunication(). ServiceRuntime::InitCommunication is a large method that does several things: - Sets up a command channel. - Loads a module (nexe). - Sets up a reverse channel. - Starts a module (nexe). This change breaks this up into smaller methods. As part of the trusted plugin refactor, we'll probably move setting up the reverse channel before loading the module, and join loading and starting the module into one step. This change should make the changes to use Chromium IPC easier to follow. BUG=333950 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245555

Patch Set 1 #

Total comments: 1

Patch Set 2 : Remove nacl_desc if check, use CHECK() instead. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -9 lines) Patch
M ppapi/native_client/src/trusted/plugin/service_runtime.h View 1 chunk +3 lines, -1 line 0 comments Download
M ppapi/native_client/src/trusted/plugin/service_runtime.cc View 1 4 chunks +25 lines, -8 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
teravest
6 years, 11 months ago (2014-01-16 21:52:55 UTC) #1
Mark Seaborn
LGTM https://codereview.chromium.org/133073005/diff/1/ppapi/native_client/src/trusted/plugin/service_runtime.cc File ppapi/native_client/src/trusted/plugin/service_runtime.cc (right): https://codereview.chromium.org/133073005/diff/1/ppapi/native_client/src/trusted/plugin/service_runtime.cc#newcode506 ppapi/native_client/src/trusted/plugin/service_runtime.cc:506: if (nacl_desc && !subprocess_->LoadModule(&command_channel_, nacl_desc)) { You might ...
6 years, 11 months ago (2014-01-16 22:02:41 UTC) #2
dmichael (off chromium)
lgtm
6 years, 11 months ago (2014-01-16 22:37:13 UTC) #3
teravest
On 2014/01/16 22:02:41, Mark Seaborn wrote: > LGTM > > https://codereview.chromium.org/133073005/diff/1/ppapi/native_client/src/trusted/plugin/service_runtime.cc > File ppapi/native_client/src/trusted/plugin/service_runtime.cc (right): ...
6 years, 11 months ago (2014-01-17 15:11:10 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/teravest@chromium.org/133073005/120001
6 years, 11 months ago (2014-01-17 15:11:26 UTC) #5
commit-bot: I haz the power
6 years, 11 months ago (2014-01-17 18:07:47 UTC) #6
Message was sent while issue was closed.
Change committed as 245555

Powered by Google App Engine
This is Rietveld 408576698