Make it possible for embedders to select the subprocesses supported by their main function |
|
Issue descriptionSome embedders may want to split their subprocesses into multiple binaries in a similar way to how chrome.dll and chrome_child.dll are built on Windows. Right now, achieving this requires building multiple copies of //content/app for each binary. A cleaner way to do it would probably be to find a way to move the references to the subprocesses into the embedder's main function.
,
Jun 21 2018
The basic way that I think this should work is: - split ContentMainDelegate into multiple classes, one for each subprocess, each derived from a ProcessRunner base class - the ProcessRunner for subprocess foo would generally live under //content/foo - each derived class would implement a Run() function (declared as pure virtual in ProcessRunner) that implements the subprocess, and declare a pure virtual CreateContent*Client() function that embedders would implement - each embedder creates a vector of ProcessRunners and passes it to ContentMain. I have a work-in-progress CL here: https://chromium-review.googlesource.com/c/chromium/src/+/1110886
,
Jun 27 2018
> each embedder creates a vector of ProcessRunners and passes it to ContentMain. Having given it some more thought I think we should do something slightly different than this. Since the common case is for ContentMain to support all subprocesses, the way that I'm currently doing things is to put the vector of ProcessRunners in ContentMainDelegate, and in general the ContentMainDelegate derived class would be responsible for filling it in. For ContentMainDelegates that might need to support different subsets of subprocesses (such as ChromeContentMainDelegate), I'm thinking that we would expose functions as part of the //chrome API that would do things like "add the browser subprocess to a ChromeContentMainDelegate" or "add the child subprocesses to a ChromeContentMainDelegate" -- then main functions would call the appropriate //chrome functions. jam: I'd appreciate any feedback that you might have on the overall approach.
,
Jun 28 2018
is there a design doc for what this is about?
,
Jun 29 2018
There's some context in this internal doc: https://docs.google.com/document/d/1gattKwIg-8M9FHveJ5VRrnY-58I2GwAtduZSRvM1J9Q/edit
,
Jun 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c0a0a104ddbef16788236d87647903460a21f9e commit 5c0a0a104ddbef16788236d87647903460a21f9e Author: Peter Collingbourne <pcc@chromium.org> Date: Fri Jun 29 21:20:21 2018 content: Remove unused declaration. Bug: 855193 Change-Id: Ic7ca91171215cc7054ca56af3ef6d496c2fcfb55 Reviewed-on: https://chromium-review.googlesource.com/1117862 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Peter Collingbourne <pcc@chromium.org> Cr-Commit-Position: refs/heads/master@{#571634} [modify] https://crrev.com/5c0a0a104ddbef16788236d87647903460a21f9e/content/app/content_main_runner_impl.cc
,
Jun 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/15bcab8bd3ac9eccfdc708841d4e945c8860ede0 commit 15bcab8bd3ac9eccfdc708841d4e945c8860ede0 Author: Peter Collingbourne <pcc@chromium.org> Date: Fri Jun 29 21:38:38 2018 service_manager: Only start the service manager thread in the browser process. The browser process is the only embedder process that needs a service manager thread. With this change we avoid spinning up an unnecessary I/O thread (and presumably, once issue 729596 is fixed, a service manager) in processes that are neither subprocesses nor the browser process, such as the cloud print service process. Bug: 855193 Change-Id: Iffae3405f6dd541ce898ac2033c9c10daefd0031 Reviewed-on: https://chromium-review.googlesource.com/1117473 Commit-Queue: Peter Collingbourne <pcc@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Cr-Commit-Position: refs/heads/master@{#571640} [modify] https://crrev.com/15bcab8bd3ac9eccfdc708841d4e945c8860ede0/services/service_manager/embedder/main.cc
,
Jul 11
Okay, https://chromium-review.googlesource.com/c/chromium/src/+/1110886 is now pretty much what I think the end state should look like. On Windows chrome.dll+chrome_child.dll sizes increase by 5KB and on Android libmonochrome.so 32+64 bit sizes shrink by 2KB (perhaps because with this change the BrowserMain function is statically unreachable on Android -- previously it was dynamically unreachable). Should there be a more detailed design doc or is the CL enough? Assuming there are no objections I'll start splitting it up into smaller pieces and upstreaming it.
,
Aug 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d9116474737152478e8183611c01c6ca732dec5c commit d9116474737152478e8183611c01c6ca732dec5c Author: Peter Collingbourne <pcc@chromium.org> Date: Wed Aug 01 16:31:12 2018 content: Remove the "empty" content clients. These don't appear to be necessary: almost all clients return non-null pointers from their CreateContent*Client functions, so we can use the content clients directly. The one exception is the chromecast utility client whose use of that functionality seems unintentional. Bug: 855193 Change-Id: I9b835de11dfe37799b30bba7921513438a0d5a9e Reviewed-on: https://chromium-review.googlesource.com/1157659 Reviewed-by: Luke Halliwell <halliwell@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Peter Collingbourne <pcc@chromium.org> Cr-Commit-Position: refs/heads/master@{#579831} [modify] https://crrev.com/d9116474737152478e8183611c01c6ca732dec5c/chromecast/utility/cast_content_utility_client_simple.cc [modify] https://crrev.com/d9116474737152478e8183611c01c6ca732dec5c/content/app/content_main_runner_impl.cc
,
Aug 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/24c818c9a9915bd0350d2149c3dc367694d89303 commit 24c818c9a9915bd0350d2149c3dc367694d89303 Author: Peter Collingbourne <pcc@chromium.org> Date: Wed Aug 01 20:38:36 2018 extensions: Remove virtual functions that are never overridden. Bug: 855193 Change-Id: I2e30bd2e6188a7695b469955c896dd5900fb1793 Reviewed-on: https://chromium-review.googlesource.com/1157545 Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Commit-Queue: Peter Collingbourne <pcc@chromium.org> Cr-Commit-Position: refs/heads/master@{#579924} [modify] https://crrev.com/24c818c9a9915bd0350d2149c3dc367694d89303/extensions/shell/app/shell_main_delegate.cc [modify] https://crrev.com/24c818c9a9915bd0350d2149c3dc367694d89303/extensions/shell/app/shell_main_delegate.h
,
Aug 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/381beab379e4243ece5610862be54367b2ed4dd6 commit 381beab379e4243ece5610862be54367b2ed4dd6 Author: Peter Collingbourne <pcc@chromium.org> Date: Thu Aug 02 19:42:51 2018 chrome: Call SetUpBundleOverrides from ChromeMain. It looks like we always need to call SetUpBundleOverrides no matter what the process type is, so we can just call it from ChromeMain. This will save us from needing to call this function from browser-specific code in a later change. While here, also remove a dead declaration. Bug: 855193 Change-Id: I449d92ef9c7d80d066d4cfefb3015014bd407cc1 Reviewed-on: https://chromium-review.googlesource.com/1159335 Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#580285} [modify] https://crrev.com/381beab379e4243ece5610862be54367b2ed4dd6/chrome/app/chrome_main.cc [modify] https://crrev.com/381beab379e4243ece5610862be54367b2ed4dd6/chrome/app/chrome_main_delegate.cc [modify] https://crrev.com/381beab379e4243ece5610862be54367b2ed4dd6/chrome/app/chrome_main_mac.h
,
Aug 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7a07960a925b2b0c2e8dc5e9b49f349fb5bd3e7a commit 7a07960a925b2b0c2e8dc5e9b49f349fb5bd3e7a Author: Kunihiko Sakamoto <ksakamoto@chromium.org> Date: Fri Aug 03 05:19:12 2018 Revert "chrome: Call SetUpBundleOverrides from ChromeMain." This reverts commit 381beab379e4243ece5610862be54367b2ed4dd6. Reason for revert: Suspected of breaking browser_tests on Mac. crbug.com/870579 Original change's description: > chrome: Call SetUpBundleOverrides from ChromeMain. > > It looks like we always need to call SetUpBundleOverrides no matter > what the process type is, so we can just call it from ChromeMain. This > will save us from needing to call this function from browser-specific > code in a later change. > > While here, also remove a dead declaration. > > Bug: 855193 > Change-Id: I449d92ef9c7d80d066d4cfefb3015014bd407cc1 > Reviewed-on: https://chromium-review.googlesource.com/1159335 > Reviewed-by: Robert Sesek <rsesek@chromium.org> > Reviewed-by: Avi Drissman <avi@chromium.org> > Commit-Queue: Avi Drissman <avi@chromium.org> > Cr-Commit-Position: refs/heads/master@{#580285} TBR=avi@chromium.org,pcc@chromium.org,rsesek@chromium.org,mark@chromium.org Change-Id: I32f905ff8f82306a85fbc9798652b8c99aa46009 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 855193,870579 Reviewed-on: https://chromium-review.googlesource.com/1161722 Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org> Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#580456} [modify] https://crrev.com/7a07960a925b2b0c2e8dc5e9b49f349fb5bd3e7a/chrome/app/chrome_main.cc [modify] https://crrev.com/7a07960a925b2b0c2e8dc5e9b49f349fb5bd3e7a/chrome/app/chrome_main_delegate.cc [modify] https://crrev.com/7a07960a925b2b0c2e8dc5e9b49f349fb5bd3e7a/chrome/app/chrome_main_mac.h
,
Aug 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/65ef9c4596407c5a063d4adae8baf6bc67aee131 commit 65ef9c4596407c5a063d4adae8baf6bc67aee131 Author: Peter Collingbourne <pcc@chromium.org> Date: Fri Aug 03 16:54:16 2018 content: Move function that initializes field trials to child. A future change will move the calls to this function into the individual subprocess implementations. Also simplify the API by leaking the field trial list instead of requiring the caller to keep it on the stack. This is similar to what we do in the browser process (see ChromeBrowserMainParts::SetupFieldTrials). Bug: 855193 Change-Id: Idf5fbd18d8a6b6f5831d7d5ef63293c22d60d127 Reviewed-on: https://chromium-review.googlesource.com/1159740 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Peter Collingbourne <pcc@chromium.org> Cr-Commit-Position: refs/heads/master@{#580572} [modify] https://crrev.com/65ef9c4596407c5a063d4adae8baf6bc67aee131/content/app/content_main_runner_impl.cc [modify] https://crrev.com/65ef9c4596407c5a063d4adae8baf6bc67aee131/content/child/BUILD.gn [add] https://crrev.com/65ef9c4596407c5a063d4adae8baf6bc67aee131/content/child/field_trial.cc [add] https://crrev.com/65ef9c4596407c5a063d4adae8baf6bc67aee131/content/child/field_trial.h
,
Aug 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ddeb4c0cccc2d8154c98f730808c95dde006e4ff commit ddeb4c0cccc2d8154c98f730808c95dde006e4ff Author: Peter Collingbourne <pcc@chromium.org> Date: Tue Aug 07 21:29:30 2018 content: Inline MachBroker::ChildSendTaskPortToParent into callers. This function lives in //content/browser despite being intended to be used by child processes. The function body is trivial and can be inlined into every caller. This lets us remove one dependency from app to browser. Bug: 855193 Change-Id: Ia9558b4298275a5638500ebcd65361073be838b9 Reviewed-on: https://chromium-review.googlesource.com/1159394 Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Commit-Queue: Peter Collingbourne <pcc@chromium.org> Cr-Commit-Position: refs/heads/master@{#581342} [modify] https://crrev.com/ddeb4c0cccc2d8154c98f730808c95dde006e4ff/content/app/content_main_runner_impl.cc [modify] https://crrev.com/ddeb4c0cccc2d8154c98f730808c95dde006e4ff/content/browser/mach_broker_mac.mm [modify] https://crrev.com/ddeb4c0cccc2d8154c98f730808c95dde006e4ff/content/browser/mach_broker_mac_unittest.cc [modify] https://crrev.com/ddeb4c0cccc2d8154c98f730808c95dde006e4ff/content/common/content_constants_internal.cc [modify] https://crrev.com/ddeb4c0cccc2d8154c98f730808c95dde006e4ff/content/common/content_constants_internal.h
,
Aug 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/29ee01681f295842fec78c0eb5db04b7a1b56270 commit 29ee01681f295842fec78c0eb5db04b7a1b56270 Author: Peter Collingbourne <pcc@chromium.org> Date: Fri Aug 10 01:41:27 2018 extensions: De-duplicate ShellCrApplication initialization. This will allow ShellMainDelegate and TestShellMainDelegate to share an implementation of the browser subprocess later. Bug: 855193 Change-Id: Id4ef662bd96b606f19b4ecedda15a8a1bbed979b Reviewed-on: https://chromium-review.googlesource.com/1168647 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Commit-Queue: Peter Collingbourne <pcc@chromium.org> Cr-Commit-Position: refs/heads/master@{#582003} [modify] https://crrev.com/29ee01681f295842fec78c0eb5db04b7a1b56270/extensions/shell/BUILD.gn [modify] https://crrev.com/29ee01681f295842fec78c0eb5db04b7a1b56270/extensions/shell/app/DEPS [modify] https://crrev.com/29ee01681f295842fec78c0eb5db04b7a1b56270/extensions/shell/app/shell_main_delegate.h [rename] https://crrev.com/29ee01681f295842fec78c0eb5db04b7a1b56270/extensions/shell/app/shell_main_delegate_mac.mm [modify] https://crrev.com/29ee01681f295842fec78c0eb5db04b7a1b56270/extensions/shell/browser/DEPS [modify] https://crrev.com/29ee01681f295842fec78c0eb5db04b7a1b56270/extensions/shell/browser/shell_browser_main_parts.cc [delete] https://crrev.com/8b8659b15ab630d1e92d20a97b7d3c2d24e295e6/extensions/shell/browser/shell_browser_main_parts_mac.h [delete] https://crrev.com/8b8659b15ab630d1e92d20a97b7d3c2d24e295e6/extensions/shell/browser/shell_browser_main_parts_mac.mm [modify] https://crrev.com/29ee01681f295842fec78c0eb5db04b7a1b56270/extensions/shell/test/test_shell_main_delegate.h |
|
►
Sign in to add a comment |
|
Comment 1 by p...@chromium.org
, Jun 21 2018