Refactor some more BrowserMain code. Move core code that's required by all embedders to content.

Refactor some more BrowserMain code. Move the small bit of remaining code that's required by all embedders to content.

BUG=90445
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=100223

[jam, 2011-09-08 01:34:27]

oops, I meant to send this to Trung

[jam, 2011-09-08 16:21:06]

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main.cc
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main...
chrome/browser/browser_main.cc:1756: 
fixed this spacing and also 1766

[jam, 2011-09-08 16:38:01]

http://codereview.chromium.org/7840041/diff/15013/content/browser/browser_mai...
File content/browser/browser_main.cc (right):

http://codereview.chromium.org/7840041/diff/15013/content/browser/browser_mai...
content/browser/browser_main.cc:247: {
I got rid of this unnecessary scope

[tony, 2011-09-08 17:21:04]

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main.cc
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main...
chrome/browser/browser_main.cc:1897: if (parameters().ui_task == NULL &&
translate_manager_ != NULL)
Do we need to make a member variable for translate_manager_ or can we just call
TranslateManager::GetInstance() again?

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main...
chrome/browser/browser_main.cc:1905: metrics_->Stop();
It looks like this is g_browser_process->metrics_service(), so we can probably
use that instead of making a member variable for it.

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main.h
File chrome/browser/browser_main.h (right):

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main...
chrome/browser/browser_main.h:14: #include
"chrome/browser/first_run/first_run.h"
Nit: alphabetize

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/chromeos/bro...
File chrome/browser/chromeos/browser_main_chromeos.cc (right):

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/chromeos/bro...
chrome/browser/chromeos/browser_main_chromeos.cc:93:
BrowserMainPartsGtk::PostMainMessageLoopStart();
Posix seems better here since there is no
BrowserMainPartsGtk::PostMainMessageLoopStart, but if we were to add one, it
would be a good place for GTK (not chromeos) specific code.

http://codereview.chromium.org/7840041/diff/15013/content/browser/browser_main.h
File content/browser/browser_main.h (right):

http://codereview.chromium.org/7840041/diff/15013/content/browser/browser_mai...
content/browser/browser_main.h:78: void MainMessageLoopRunWrapper();
Nit: I would probably name this RunMainMessageLoopParts or something.  Wrapper
isn't very descriptive.

[jam, 2011-09-08 17:43:01]

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main.cc
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main...
chrome/browser/browser_main.cc:1897: if (parameters().ui_task == NULL &&
translate_manager_ != NULL)
On 2011/09/08 17:21:04, tony wrote:
> Do we need to make a member variable for translate_manager_ or can we just
call
> TranslateManager::GetInstance() again?

I had done this so that we don't end up constructing TranslateManager in
PostMainMessageLoopRun() if it wasn't constructed before (to match the old
behavior)

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main...
chrome/browser/browser_main.cc:1905: metrics_->Stop();
On 2011/09/08 17:21:04, tony wrote:
> It looks like this is g_browser_process->metrics_service(), so we can probably
> use that instead of making a member variable for it.

Done.

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main.h
File chrome/browser/browser_main.h (right):

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/browser_main...
chrome/browser/browser_main.h:14: #include
"chrome/browser/first_run/first_run.h"
On 2011/09/08 17:21:04, tony wrote:
> Nit: alphabetize

Done.

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/chromeos/bro...
File chrome/browser/chromeos/browser_main_chromeos.cc (right):

http://codereview.chromium.org/7840041/diff/15013/chrome/browser/chromeos/bro...
chrome/browser/chromeos/browser_main_chromeos.cc:93:
BrowserMainPartsGtk::PostMainMessageLoopStart();
On 2011/09/08 17:21:04, tony wrote:
> Posix seems better here since there is no
> BrowserMainPartsGtk::PostMainMessageLoopStart, but if we were to add one, it
> would be a good place for GTK (not chromeos) specific code.

ok. it seemed odd to me that it wouldn't call its parent class, but what you say
makes sense

http://codereview.chromium.org/7840041/diff/15013/content/browser/browser_main.h
File content/browser/browser_main.h (right):

http://codereview.chromium.org/7840041/diff/15013/content/browser/browser_mai...
content/browser/browser_main.h:78: void MainMessageLoopRunWrapper();
On 2011/09/08 17:21:04, tony wrote:
> Nit: I would probably name this RunMainMessageLoopParts or something.  Wrapper
> isn't very descriptive.

Done.

[tony, 2011-09-08 18:37:26]

The net_unittests failure looks real, no?

[tony, 2011-09-08 19:33:38]

On 2011/09/08 18:37:26, tony wrote:
> The net_unittests failure looks real, no?

Nevermind, looks like agl is looking into that failure.

LGTM