Issue 2931007: BrowserMain() refactoring, part 2.

Issue 2931007: BrowserMain() refactoring, part 2. (Closed)

Created:
10 years, 5 months ago by viettrungluu

Modified:
9 years, 6 months ago

Reviewers:
brettw

CC:
chromium-reviews, John Grabowski, ben+cc_chromium.org, pam+watch_chromium.org, brettw-cc_chromium.org

Visibility:
Public.

Description

BrowserMain() refactoring, part 2. Add "MainMessageLoopStart()" and related platform methods to handle tasks directly tied to the start of the main message loop. BUG=none TEST=everything still works Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=53783

Patch Set 1 #

Patch Set 2 : foo #

Patch Set 3 : rebased ToT #

Patch Set 4 : merged ToT #

Patch Set 5 : oops #

Patch Set 6 : oops^2 #

Patch Set 7 : remove FIXME #

Total comments: 7

Patch Set 8 : Updated per brettw and merged ToT. #

Created: 10 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+225 lines, -161 lines)			Patch
M	chrome/browser/browser_main.h	View	1 2 3 4 5 6 7	6 chunks	+33 lines, -11 lines	0 comments	Download
M	chrome/browser/browser_main.cc	View	1 2 3 4 5 6 7	4 chunks	+39 lines, -30 lines	0 comments	Download
M	chrome/browser/browser_main_gtk.cc	View		1 chunk	+0 lines, -3 lines	0 comments	Download
M	chrome/browser/browser_main_mac.mm	View	1 2 3 5	3 chunks	+67 lines, -49 lines	0 comments	Download
A	chrome/browser/browser_main_posix.h	View		1 chunk	+20 lines, -0 lines	0 comments	Download
M	chrome/browser/browser_main_posix.cc	View	1 2 3 4 5 6 7	3 chunks	+60 lines, -63 lines	0 comments	Download
M	chrome/browser/browser_main_win.cc	View	1 2 3 4 5	2 chunks	+5 lines, -5 lines	0 comments	Download
M	chrome/chrome_browser.gypi	View	1 2 3 4 5 6 7	1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

brettw

LGTM http://codereview.chromium.org/2931007/diff/13001/14001 File chrome/browser/browser_main.cc (right): http://codereview.chromium.org/2931007/diff/13001/14001#newcode328 chrome/browser/browser_main.cc:328: // TODO(viettrungluu): should these really go before setting ...

10 years, 5 months ago (2010-07-26 17:03:37 UTC) #2

viettrungluu

10 years, 5 months ago (2010-07-27 05:02:10 UTC) #3

Thanks.

http://codereview.chromium.org/2931007/diff/13001/14001
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/2931007/diff/13001/14001#newcode328
chrome/browser/browser_main.cc:328: // TODO(viettrungluu): should these really
go before setting the thread name
On 2010/07/26 17:03:37, brettw wrote:
> Punctuation at the end of comments.

Done.

http://codereview.chromium.org/2931007/diff/13001/14001#newcode335
chrome/browser/browser_main.cc:335: // TODO(viettrungluu): more stuff
On 2010/07/26 17:03:37, brettw wrote:
> It's not clear to me what "more stuff" should go here. Can you either be
> explicit about what you think should be moved here or just delete this
comment?

Deleted.

http://codereview.chromium.org/2931007/diff/13001/14002
File chrome/browser/browser_main.h (right):

http://codereview.chromium.org/2931007/diff/13001/14002#newcode114
chrome/browser/browser_main.h:114: scoped_ptr<MessageLoop> main_message_loop_;
On 2010/07/26 17:03:37, brettw wrote:
> You should be able to forward-declare all of the classes you put into these
> scoped_ptrs. The requirement is that the classes be defined when the
> constructors and destructor are compiled (i.e. they should be in the .cc
file).

Right. My mistake was not giving this class an explicit destructor, which meant
that subclasses also needed the full definitions.

Expand Messages | Collapse Messages