Upstreaming ChromeBrowserMainAndroid

Upstreaming ChromeBrowserMainAndroid

Upstreaming the chrome_browser_main_android and its immediate dependencies. This code is used to override the default chromium initialization with android specific initialization. Android uses custom MessageLoop.

BUG=138184
TEST=Compile unit_tests_apk target


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

[aurimas (slooooooooow), 2012-07-24 21:27:44]

Hello Nico,

Could you take a look at my CL when you get a chance? I am upstreaming Chrome
for Android code.

Thanks,
Aurimas

[Nico, 2012-07-24 21:32:49]

Can you change the CL description so that it describes what the new code does,
instead of just saying "upstreaming"?

[Nico, 2012-07-24 21:33:30]

https://chromiumcodereview.appspot.com/10819003/diff/1/chrome/chrome_browser....
File chrome/chrome_browser.gypi (right):

https://chromiumcodereview.appspot.com/10819003/diff/1/chrome/chrome_browser....
chrome/chrome_browser.gypi:405: 'browser/chrome_browser_main_android.h',
You don't add the _adapter_ files anywhere. Is this intentional?

[aurimas (slooooooooow), 2012-07-24 22:27:13]

Hello again,

I removed the android_protocol_adapter for now and it will be added as another
CL. I also added a small description of what this CL does.

Thanks,
Aurimas

https://chromiumcodereview.appspot.com/10819003/diff/1/chrome/chrome_browser....
File chrome/chrome_browser.gypi (right):

https://chromiumcodereview.appspot.com/10819003/diff/1/chrome/chrome_browser....
chrome/chrome_browser.gypi:405: 'browser/chrome_browser_main_android.h',
On 2012/07/24 21:33:30, Nico wrote:
> You don't add the _adapter_ files anywhere. Is this intentional?
It was a mistake, but when I fixed it, I realized that there a lot of
dependencies with android_protocol_adapter. I will separate that to a separate
CL.

[Nico, 2012-07-25 19:50:16]

http://codereview.chromium.org/10819003/diff/1/chrome/browser/chrome_browser_...
File chrome/browser/chrome_browser_main_android.cc (right):

http://codereview.chromium.org/10819003/diff/1/chrome/browser/chrome_browser_...
chrome/browser/chrome_browser_main_android.cc:59: // Not implemented for now.
Do you plan to implement this eventually?

http://codereview.chromium.org/10819003/diff/1/chrome/browser/chrome_browser_...
File chrome/browser/chrome_browser_main_android.h (right):

http://codereview.chromium.org/10819003/diff/1/chrome/browser/chrome_browser_...
chrome/browser/chrome_browser_main_android.h:23: scoped_ptr<MessageLoop>
main_message_loop_;
DISALLOW_COPY_AND_ASSIGN?

http://codereview.chromium.org/10819003/diff/5001/chrome/browser/chrome_brows...
File chrome/browser/chrome_browser_main_android.cc (right):

http://codereview.chromium.org/10819003/diff/5001/chrome/browser/chrome_brows...
chrome/browser/chrome_browser_main_android.cc:31: const std::string gcov_prefix
=
std::string&

http://codereview.chromium.org/10819003/diff/5001/chrome/browser/chrome_brows...
chrome/browser/chrome_browser_main_android.cc:35: if
(parsed_command_line().HasSwitch(switches::kGcovPrefixStrip)) {
This looks identical to the previous if. Do you need both?

http://codereview.chromium.org/10819003/diff/5001/chrome/browser/chrome_brows...
chrome/browser/chrome_browser_main_android.cc:46:
MessageLoopForUI::current()->Start();
content/shell/shell_browser_main_parts.cc does this in PostMessageLoopStart().
Should this match, for consistency?

Also, a comment why this is done in the Parts object in android would be nice.

http://codereview.chromium.org/10819003/diff/5001/chrome/browser/chrome_brows...
chrome/browser/chrome_browser_main_android.cc:52: VLOG(0) <<
"ShowMissingLocaleMessageBox";
Shoudl this be NOTREACHED() instead?

http://codereview.chromium.org/10819003/diff/5001/chrome/browser/chrome_brows...
chrome/browser/chrome_browser_main_android.cc:56: // Not implemented for now.
If you intend to implement this, file a tracking bug and mention it here. Else,
remove comment.

http://codereview.chromium.org/10819003/diff/5001/chrome/browser/chrome_brows...
chrome/browser/chrome_browser_main_android.cc:60: // Nothing to warn right now.
Remove comment.

[aurimas (slooooooooow), 2012-07-25 22:05:46]

Hello Nico,

I have followed your suggestions and created a new patch. Please take a look at
it when you get a chance.

Aurimas

https://chromiumcodereview.appspot.com/10819003/diff/1/chrome/browser/chrome_...
File chrome/browser/chrome_browser_main_android.cc (right):

https://chromiumcodereview.appspot.com/10819003/diff/1/chrome/browser/chrome_...
chrome/browser/chrome_browser_main_android.cc:59: // Not implemented for now.
On 2012/07/25 19:50:16, Nico wrote:
> Do you plan to implement this eventually?

Done.

https://chromiumcodereview.appspot.com/10819003/diff/1/chrome/browser/chrome_...
File chrome/browser/chrome_browser_main_android.h (right):

https://chromiumcodereview.appspot.com/10819003/diff/1/chrome/browser/chrome_...
chrome/browser/chrome_browser_main_android.h:23: scoped_ptr<MessageLoop>
main_message_loop_;
On 2012/07/25 19:50:16, Nico wrote:
> DISALLOW_COPY_AND_ASSIGN?

Done.

https://chromiumcodereview.appspot.com/10819003/diff/5001/chrome/browser/chro...
File chrome/browser/chrome_browser_main_android.cc (right):

https://chromiumcodereview.appspot.com/10819003/diff/5001/chrome/browser/chro...
chrome/browser/chrome_browser_main_android.cc:31: const std::string gcov_prefix
=
On 2012/07/25 19:50:16, Nico wrote:
> std::string&

Done.

https://chromiumcodereview.appspot.com/10819003/diff/5001/chrome/browser/chro...
chrome/browser/chrome_browser_main_android.cc:35: if
(parsed_command_line().HasSwitch(switches::kGcovPrefixStrip)) {
Completely removing GCOV as it is not actually being used.

https://chromiumcodereview.appspot.com/10819003/diff/5001/chrome/browser/chro...
chrome/browser/chrome_browser_main_android.cc:46:
MessageLoopForUI::current()->Start();
On 2012/07/25 19:50:16, Nico wrote:
> content/shell/shell_browser_main_parts.cc does this in PostMessageLoopStart().
> Should this match, for consistency?
> 
> Also, a comment why this is done in the Parts object in android would be nice.

Done.

https://chromiumcodereview.appspot.com/10819003/diff/5001/chrome/browser/chro...
chrome/browser/chrome_browser_main_android.cc:52: VLOG(0) <<
"ShowMissingLocaleMessageBox";
On 2012/07/25 19:50:16, Nico wrote:
> Shoudl this be NOTREACHED() instead?

Done.

https://chromiumcodereview.appspot.com/10819003/diff/5001/chrome/browser/chro...
chrome/browser/chrome_browser_main_android.cc:56: // Not implemented for now.
On 2012/07/25 19:50:16, Nico wrote:
> If you intend to implement this, file a tracking bug and mention it here.
Else,
> remove comment.

Done.

https://chromiumcodereview.appspot.com/10819003/diff/5001/chrome/browser/chro...
chrome/browser/chrome_browser_main_android.cc:60: // Nothing to warn right now.
On 2012/07/25 19:50:16, Nico wrote:
> Remove comment.

Done.

[Nico, 2012-07-25 22:17:43]

lgtm with comments

Can you also remove the if OS_ANDROID bit from
chrome/browser/chrome_browser_main_posix.cc now that it looks that _posix.cc
won't be used on android?

https://chromiumcodereview.appspot.com/10819003/diff/9001/chrome/common/chrom...
File chrome/common/chrome_switches.cc (right):

https://chromiumcodereview.appspot.com/10819003/diff/9001/chrome/common/chrom...
chrome/common/chrome_switches.cc:1352: const char kGcovPrefixStrip[]            
  = "gcov-prefix-strip";
ditto

https://chromiumcodereview.appspot.com/10819003/diff/9001/chrome/common/chrom...
File chrome/common/chrome_switches.h (right):

https://chromiumcodereview.appspot.com/10819003/diff/9001/chrome/common/chrom...
chrome/common/chrome_switches.h:363: extern const char kGcovPrefixStrip[];
this can be reverted now, too

[aurimas (slooooooooow), 2012-08-01 20:31:40]

Hello Nico,

I fixed the issues you mentioned.

Aurimas

[Nico, 2012-08-01 20:35:12]

lgtm

[commit-bot: I haz the power, 2012-08-07 21:14:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aurimas@chromium.org/10819003/21001

[commit-bot: I haz the power, 2012-08-07 22:46:35]

Change committed as 150438