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

Issue 9968053: Refactor ProcessSingleton so that it may be used distinctly from a full browser process. (Closed)

Created:
8 years, 8 months ago by erikwright (departed)
Modified:
8 years, 8 months ago
Reviewers:
robertshield, jam
CC:
chromium-reviews
Visibility:
Public.

Description

Refactor ProcessSingleton so that it may be used distinctly from a full browser process. The main effect is that ProcessSingleton is much more restricted to its basic functionality, which is to ensure that a single instance of Chrome runs per profile directory and to pass messages from new Chrome invocations to the existing instance. Exactly how those messages were handled has been moved from the implementations of ProcessSingleton into ChromeBrowserMain where (I think) it more rightly belongs. This will allow the Chrome Frame net tests to use ProcessSingleton to implement a stub Chrome for the purpose of handling network-related IPC from Chrome Frame without having to launch the rest of Chrome, upon which ProcessSingleton previously depended directly. BUG=None TEST=None Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=130853

Patch Set 1 #

Patch Set 2 : Fix a call to ProcessCommandLine #

Patch Set 3 : Move callback declaration out of windows ifdef. #

Patch Set 4 : Fix build break on Linux. #

Patch Set 5 : Fix mac build breakage. #

Patch Set 6 : Mac unit-test fixage. #

Patch Set 7 : Linux unit-test fixage. #

Patch Set 8 : Further linux test compilation fixage. #

Patch Set 9 : Merge master. #

Patch Set 10 : Include cleanup. #

Total comments: 9

Patch Set 11 : Remove legacy code that doesn't need to survive this refactoring. Fix a comment. #

Patch Set 12 : Fix comment nits. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+128 lines, -122 lines) Patch
M chrome/browser/chrome_browser_main.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +35 lines, -1 line 0 comments Download
M chrome/browser/process_singleton.h View 1 2 3 4 5 6 7 8 9 10 11 7 chunks +21 lines, -7 lines 0 comments Download
M chrome/browser/process_singleton.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/process_singleton_linux.cc View 1 2 3 4 5 6 7 8 9 7 chunks +14 lines, -38 lines 0 comments Download
M chrome/browser/process_singleton_linux_uitest.cc View 1 2 3 4 5 6 7 8 9 10 11 6 chunks +19 lines, -4 lines 0 comments Download
M chrome/browser/process_singleton_mac.cc View 1 2 3 4 2 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/process_singleton_mac_unittest.cc View 1 2 3 4 5 4 chunks +18 lines, -10 lines 0 comments Download
M chrome/browser/process_singleton_win.cc View 5 chunks +13 lines, -54 lines 0 comments Download

Messages

Total messages: 18 (0 generated)
erikwright (departed)
PTAL! Thanks, Erik
8 years, 8 months ago (2012-04-03 19:18:20 UTC) #1
jam
the patch is fine, but I'm not following what instantiating ProcessSingleton has to do with ...
8 years, 8 months ago (2012-04-04 00:42:58 UTC) #2
jam
btw I didn't write ProcessSingleton, so it might be best to find the original author's ...
8 years, 8 months ago (2012-04-04 00:43:33 UTC) #3
jam
http://codereview.chromium.org/9968053/diff/5020/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): http://codereview.chromium.org/9968053/diff/5020/chrome/browser/chrome_browser_main.cc#newcode548 chrome/browser/chrome_browser_main.cc:548: PrefService* prefs = g_browser_process->local_state(); nit: this isn't used
8 years, 8 months ago (2012-04-04 00:44:44 UTC) #4
robertshield
LGTM, just nits from me. IIRC the intent here is that a test exe will ...
8 years, 8 months ago (2012-04-04 01:07:15 UTC) #5
erikwright (departed)
On 2012/04/04 01:07:15, robertshield wrote: > LGTM, just nits from me. > > IIRC the ...
8 years, 8 months ago (2012-04-04 02:40:23 UTC) #6
erikwright (departed)
On 2012/04/04 00:42:58, John Abd-El-Malek wrote: > the patch is fine, but I'm not following ...
8 years, 8 months ago (2012-04-04 02:48:03 UTC) #7
erikwright (departed)
On 2012/04/04 00:43:33, John Abd-El-Malek wrote: > btw I didn't write ProcessSingleton, so it might ...
8 years, 8 months ago (2012-04-04 02:50:54 UTC) #8
erikwright (departed)
http://codereview.chromium.org/9968053/diff/5020/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): http://codereview.chromium.org/9968053/diff/5020/chrome/browser/chrome_browser_main.cc#newcode548 chrome/browser/chrome_browser_main.cc:548: PrefService* prefs = g_browser_process->local_state(); On 2012/04/04 00:44:44, John Abd-El-Malek ...
8 years, 8 months ago (2012-04-04 02:51:48 UTC) #9
robertshield
On Tue, Apr 3, 2012 at 10:40 PM, <erikwright@chromium.org> wrote: > On 2012/04/04 01:07:15, robertshield ...
8 years, 8 months ago (2012-04-04 03:07:29 UTC) #10
erikwright (departed)
I spoke with stevet and pinged estade about the --uninstall-extension and kProductVersion blocks. I'm pretty ...
8 years, 8 months ago (2012-04-04 14:51:28 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikwright@chromium.org/9968053/8002
8 years, 8 months ago (2012-04-04 14:51:46 UTC) #12
jam
On 2012/04/04 02:48:03, erikwright wrote: > On 2012/04/04 00:42:58, John Abd-El-Malek wrote: > > the ...
8 years, 8 months ago (2012-04-04 15:52:29 UTC) #13
jam
lgtm
8 years, 8 months ago (2012-04-04 20:49:36 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikwright@chromium.org/9968053/8002
8 years, 8 months ago (2012-04-04 20:50:15 UTC) #15
commit-bot: I haz the power
Try job failure for 9968053-8002 (previous was lost) (retry) on win_rel for step "browser_tests". It's ...
8 years, 8 months ago (2012-04-05 01:30:37 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikwright@chromium.org/9968053/8002
8 years, 8 months ago (2012-04-05 01:37:42 UTC) #17
commit-bot: I haz the power
8 years, 8 months ago (2012-04-05 07:15:42 UTC) #18
Change committed as 130853

Powered by Google App Engine
This is Rietveld 408576698