New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 646833 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Let service process also behave as a Service connected with Shell

Project Member Reported by leon....@intel.com, Sep 14 2016

Issue description

Hi, Ken,

While reading code I came across this TODO: https://cs.chromium.org/chromium/src/content/child/child_thread_impl.cc?rcl=0&l=416 and tried to resolve it. 
I found what we need to do is just to remove kMojoChannelToken usage between service process and service utility process, then I started investigation and found more:

1, 
Currently ChannelMojo is disabled between service process <--> service utility process, because of https://cs.chromium.org/chromium/src/content/child/child_thread_impl.cc?rcl=0&l=298 , for which maybe we could just change to bellowing for now?
    use_mojo_channel = HasSwitch(kMojoApplicationChannelToken) || HasSwitch(kMojoChannelToken);

2,
To solve this issue completely, I think we need to introduce both service process and service utility process into the Service/Shell world just like render process or plugin process. # Initial idea: let ServiceProcessControl leverage BrowserChildProcessHost{Impl} to control service process.

Maybe have said some non-sense, but WDYT? Thanks!
 

Comment 1 by roc...@chromium.org, Sep 15 2016

Service processes are complicated because they don't share a common ancestor with other Chrome processes. sammc@ has already done work to get service processes using ChannelMojo in general. I don't recall what the status is for service process <-> service utility process.

I don't think we should make any decisions yet regarding service processes in the ServiceManager world. In the limit, ServiceManager itself should be a service process, and we would probably add a manifest capability to allow other services to be registered/launched as service processes themselves. All of that is probably pretty far off still, since it doesn't block any of the initial servicification work.

Comment 2 by roc...@chromium.org, Sep 15 2016

Cc: sa...@chromium.org
+sammc who can speak in more detail about status of service processes wrt ChannelMojo

Comment 3 by sa...@chromium.org, Sep 22 2016

Service <-> service utility was converted to use ChannelMojo in https://codereview.chromium.org/2084453002.

Comment 4 by leon....@intel.com, Sep 22 2016

Yeah but I suppose it's currently disabled because:
1,
Service process starts service utility process with a command line containing only kMojoChannelToken switch but no kMojoApplicationChannelToken, because they are not in ServiceManager world now.
2,
Here https://cs.chromium.org/chromium/src/content/child/child_thread_impl.cc?rcl=1474500118&l=298 service utility process will disable ChannelMojo because of lack of kMojoApplicationChannelToken switch.

So I suggest to set ChildThreadImpl::Options::use_mojo_channel as HasSwitch(kMojoApplicationChannelToken) || HasSwitch(kMojoChannelToken) firstly.
Is my understanding right..? Thanks.

Comment 5 by sa...@chromium.org, Sep 22 2016

I think you're right; service utility probably regressed at some point. I wonder if this could be causing  issue 646585 .
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3cb38fd23571822bf08c79d90a0ec49ea486492a

commit 3cb38fd23571822bf08c79d90a0ec49ea486492a
Author: leon.han <leon.han@intel.com>
Date: Fri Sep 23 03:28:34 2016

Re-enable ChannelMojo between Service process <-> service utility process.

ChannelMojo between Service process <-> service utility process is
currently disabled because:
  -- Service process starts service utility process with a command line
     containing kMojoChannelToken switch but no kMojoApplicationChannelToken,
     because they are not in ServiceManager world now.
  -- But, service utility process(ChildThreadImpl) will disable ChannelMojo
     because of lack of kMojoApplicationChannelToken switch.

This CL lets ChildThreadImpl enable ChannelMojo also for a kMojoChannelToken
switch.

BUG= 646833 
TBR=ben@chromium.org

Review-Url: https://codereview.chromium.org/2366443004
Cr-Commit-Position: refs/heads/master@{#420566}

[modify] https://crrev.com/3cb38fd23571822bf08c79d90a0ec49ea486492a/content/child/child_thread_impl.cc

Comment 7 by roc...@chromium.org, Sep 23 2016

FYI that change did in fact fix  issue 646585 . I've merged it back to 2840 branch.
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 23 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/93796f6bee4a682c742639a33d98fb836e3db20a

commit 93796f6bee4a682c742639a33d98fb836e3db20a
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Sep 23 22:19:48 2016

Re-enable ChannelMojo between Service process <-> service utility process.

ChannelMojo between Service process <-> service utility process is
currently disabled because:
  -- Service process starts service utility process with a command line
     containing kMojoChannelToken switch but no kMojoApplicationChannelToken,
     because they are not in ServiceManager world now.
  -- But, service utility process(ChildThreadImpl) will disable ChannelMojo
     because of lack of kMojoApplicationChannelToken switch.

This CL lets ChildThreadImpl enable ChannelMojo also for a kMojoChannelToken
switch.

BUG= 646833 
TBR=ben@chromium.org

Review-Url: https://codereview.chromium.org/2366443004
Cr-Commit-Position: refs/heads/master@{#420566}
(cherry picked from commit 3cb38fd23571822bf08c79d90a0ec49ea486492a)

Review URL: https://codereview.chromium.org/2367113002 .

Cr-Commit-Position: refs/branch-heads/2840@{#517}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/93796f6bee4a682c742639a33d98fb836e3db20a/content/child/child_thread_impl.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/93796f6bee4a682c742639a33d98fb836e3db20a

commit 93796f6bee4a682c742639a33d98fb836e3db20a
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Sep 23 22:19:48 2016

Re-enable ChannelMojo between Service process <-> service utility process.

ChannelMojo between Service process <-> service utility process is
currently disabled because:
  -- Service process starts service utility process with a command line
     containing kMojoChannelToken switch but no kMojoApplicationChannelToken,
     because they are not in ServiceManager world now.
  -- But, service utility process(ChildThreadImpl) will disable ChannelMojo
     because of lack of kMojoApplicationChannelToken switch.

This CL lets ChildThreadImpl enable ChannelMojo also for a kMojoChannelToken
switch.

BUG= 646833 
TBR=ben@chromium.org

Review-Url: https://codereview.chromium.org/2366443004
Cr-Commit-Position: refs/heads/master@{#420566}
(cherry picked from commit 3cb38fd23571822bf08c79d90a0ec49ea486492a)

Review URL: https://codereview.chromium.org/2367113002 .

Cr-Commit-Position: refs/branch-heads/2840@{#517}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/93796f6bee4a682c742639a33d98fb836e3db20a/content/child/child_thread_impl.cc

Components: Internals>Services>ServiceManager
Bulk applying component Internals>Services>ServiceManager to issues referencing the text ServiceManager.  This may not be 100% accurate, so please feel free to pull the component as needed.
Status: Assigned (was: Untriaged)
Status: Fixed (was: Assigned)
As far as I can tell the intent of this bug was long ago addressed by service manager integration into content.

Sign in to add a comment