Minimal number of if-def changes to support OS_IOS in content_main_runner.cc

Minimal number of if-def changes to support OS_IOS in content_main_runner.cc

BUG=None


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

[jam, 2012-09-04 19:38:49]

https://chromiumcodereview.appspot.com/10907062/diff/1/content/app/content_ma...
File content/app/content_main_runner.cc (right):

https://chromiumcodereview.appspot.com/10907062/diff/1/content/app/content_ma...
content/app/content_main_runner.cc:352: int RunNamedProcessTypeMain(
nit: i would just put this whole method behind an ifdef, since it's odd to have
this on ios even though it doesn't make sense.

https://chromiumcodereview.appspot.com/10907062/diff/1/content/app/content_ma...
content/app/content_main_runner.cc:487: setlocale(LC_ALL, "");
do you really need this on ios? if not, then just make this

#if !defined(OS_ANDROID) && !defined(OS_IOS)

https://chromiumcodereview.appspot.com/10907062/diff/1/content/app/content_ma...
content/app/content_main_runner.cc:513: #if !defined(OS_ANDROID) &&
!defined(OS_IOS)
i would then combined this with the ifdef at 484 (i.e move 505-509 below this

[leng, 2012-09-05 11:57:10]

Thanks for the feedback!

https://chromiumcodereview.appspot.com/10907062/diff/1/content/app/content_ma...
File content/app/content_main_runner.cc (right):

https://chromiumcodereview.appspot.com/10907062/diff/1/content/app/content_ma...
content/app/content_main_runner.cc:352: int RunNamedProcessTypeMain(
On 2012/09/04 19:38:49, John Abd-El-Malek wrote:
> nit: i would just put this whole method behind an ifdef, since it's odd to
have
> this on ios even though it doesn't make sense.

Done.

https://chromiumcodereview.appspot.com/10907062/diff/1/content/app/content_ma...
content/app/content_main_runner.cc:487: setlocale(LC_ALL, "");
On 2012/09/04 19:38:49, John Abd-El-Malek wrote:
> do you really need this on ios? if not, then just make this
> 
> #if !defined(OS_ANDROID) && !defined(OS_IOS)
 
All our tests passed without it.
Done.

https://chromiumcodereview.appspot.com/10907062/diff/1/content/app/content_ma...
content/app/content_main_runner.cc:513: #if !defined(OS_ANDROID) &&
!defined(OS_IOS)
On 2012/09/04 19:38:49, John Abd-El-Malek wrote:
> i would then combined this with the ifdef at 484 (i.e move 505-509 below this

Done.

https://chromiumcodereview.appspot.com/10907062/diff/2004/content/app/content...
File content/app/content_main_runner.cc (right):

https://chromiumcodereview.appspot.com/10907062/diff/2004/content/app/content...
content/app/content_main_runner.cc:661: #if !defined(OS_IOS)
Had to add this because the whole function is if-def'd out for OS_IOS.

[jam, 2012-09-05 15:34:15]

lgtm with nits

https://codereview.chromium.org/10907062/diff/2004/content/app/content_main_r...
File content/app/content_main_runner.cc (right):

https://codereview.chromium.org/10907062/diff/2004/content/app/content_main_r...
content/app/content_main_runner.cc:357: // None of the *Main processes exist on
iOS.
nit: get rid of this added line

https://codereview.chromium.org/10907062/diff/2004/content/app/content_main_r...
content/app/content_main_runner.cc:495: // The exit manager is in charge of
calling the dtors of singleton objects.
nit: I would put this line first in the comment, otherwise it's a little
confusing to talk about it and then explain it

[leng, 2012-09-06 08:43:18]

https://codereview.chromium.org/10907062/diff/2004/content/app/content_main_r...
File content/app/content_main_runner.cc (right):

https://codereview.chromium.org/10907062/diff/2004/content/app/content_main_r...
content/app/content_main_runner.cc:357: // None of the *Main processes exist on
iOS.
On 2012/09/05 15:34:15, John Abd-El-Malek wrote:
> nit: get rid of this added line

Done.

https://codereview.chromium.org/10907062/diff/2004/content/app/content_main_r...
content/app/content_main_runner.cc:495: // The exit manager is in charge of
calling the dtors of singleton objects.
On 2012/09/05 15:34:15, John Abd-El-Malek wrote:
> nit: I would put this line first in the comment, otherwise it's a little
> confusing to talk about it and then explain it

Done.

[commit-bot: I haz the power, 2012-09-06 08:43:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leng@chromium.org/10907062/3003

[commit-bot: I haz the power, 2012-09-07 00:38:51]

Try job failure for 10907062-3003 (retry) (retry) (retry) on win_rel for step
"runhooks".
It's a second try, previously, steps "interactive_ui_tests, browser_tests,
nacl_integration, sync_integration_tests, content_browsertests,
chrome_frame_net_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-09-07 07:53:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leng@chromium.org/10907062/3003

[commit-bot: I haz the power, 2012-09-07 09:42:14]

Try job failure for 10907062-3003 (retry) on mac_rel for step
"sync_integration_tests".
It's a second try, previously, step "sync_integration_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-09-07 17:11:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leng@chromium.org/10907062/3003

[commit-bot: I haz the power, 2012-09-07 18:33:06]

Try job failure for 10907062-3003 (previous was lost) (retry) on win for step
"runhooks" (clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-09-11 08:15:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leng@chromium.org/10907062/2008

[commit-bot: I haz the power, 2012-09-11 09:58:16]

Change committed as 155980