Content Shell: Introduce LayoutTestBrowserMain.

Content Shell: Introduce LayoutTestBrowserMain.

Most of ShellBrowserMain's implementation was layout test-specific. This
patch moves all of that to LayoutTestBrowserMain, and adjusts the
delegate to call that function when DRT is enabled.

BUG=420994
NOTRY=true

Committed: https://crrev.com/88e23180127784b736fa6b1d105d324add1d8151
Cr-Commit-Position: refs/heads/master@{#299879}

[Mike West, 2014-10-14 10:36:32]

mkwst@chromium.org changed reviewers:
+ jochen@chromium.org

[Mike West, 2014-10-14 10:36:33]

Another another one. Depends on https://codereview.chromium.org/651883002/.

[jochen (gone - plz use gerrit), 2014-10-14 14:52:24]

https://codereview.chromium.org/649223004/diff/1/content/shell/app/shell_main...
File content/shell/app/shell_main_delegate.cc (right):

https://codereview.chromium.org/649223004/diff/1/content/shell/app/shell_main...
content/shell/app/shell_main_delegate.cc:258: :
ShellBrowserMain(main_function_params, browser_runner_);
can the else part be inlined? the ShellBrowserMain does nothing, except return
browser_runner_.Run(), no?

[Mike West, 2014-10-15 07:31:22]

On 2014/10/14 14:52:24, jochen wrote:
>
https://codereview.chromium.org/649223004/diff/1/content/shell/app/shell_main...
> File content/shell/app/shell_main_delegate.cc (right):
> 
>
https://codereview.chromium.org/649223004/diff/1/content/shell/app/shell_main...
> content/shell/app/shell_main_delegate.cc:258: :
> ShellBrowserMain(main_function_params, browser_runner_);
> can the else part be inlined? the ShellBrowserMain does nothing, except return
> browser_runner_.Run(), no?

Ignoring error checking, it'd be something like:

---
int exit_code = browser_runner_->Initialize(main_function_params);
exit_code = browser_runner_->Run();
browser_runner_->Shutdown();
return exit_code;
---

I could certainly inline that if you'd prefer, but it's not a one-liner.

[Mike West, 2014-10-15 07:33:30]

On 2014/10/15 07:31:22, Mike West wrote:
> On 2014/10/14 14:52:24, jochen wrote:
> >
>
https://codereview.chromium.org/649223004/diff/1/content/shell/app/shell_main...
> > File content/shell/app/shell_main_delegate.cc (right):
> > 
> >
>
https://codereview.chromium.org/649223004/diff/1/content/shell/app/shell_main...
> > content/shell/app/shell_main_delegate.cc:258: :
> > ShellBrowserMain(main_function_params, browser_runner_);
> > can the else part be inlined? the ShellBrowserMain does nothing, except
return
> > browser_runner_.Run(), no?
> 
> Ignoring error checking, it'd be something like:
> 
> ---
> int exit_code = browser_runner_->Initialize(main_function_params);
> exit_code = browser_runner_->Run();
> browser_runner_->Shutdown();
> return exit_code;
> ---
> 
> I could certainly inline that if you'd prefer, but it's not a one-liner.

Oh, also, Run only gets called on Android if we're running layout tests. It's
not clear to me that that's the correct behavior, but that's what's currently
implemented.

[jochen (gone - plz use gerrit), 2014-10-15 15:27:11]

lgtm

[Mike West, 2014-10-15 15:38:18]

On 2014/10/15 15:27:11, jochen wrote:
> lgtm

Pasting in your response from the other CL, as I think it was meant to go here:

---

fair enough

lgtm

on android, we don't controll the main message loop from C++. however, for
layout tests, we just run a main message loop in a single task (which is kinda
cheating, and one of the reasons, layout tests on android don't work that well)

[Mike West, 2014-10-16 11:48:32]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-10-16 11:48:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/649223004/60001

[commit-bot: I haz the power, 2014-10-16 11:55:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-16 11:55:17]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Mike West, 2014-10-16 12:00:06]

On 2014/10/16 11:55:17, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Ugh. I'm going to call both of these false positives, as I'm just moving
existing bad code from one place to another.

That said, the header bit does look weird: +marja. :)

---

** Presubmit Messages **
Your #include order seems to be broken. Send mail to
marja@chromium.org if this is not the case.
      content/shell/browser/layout_test/layout_test_browser_main.cc:7 \
      content/shell/browser/layout_test/layout_test_browser_main.cc:9

** Presubmit ERRORS **
Banned functions were used.
    content/shell/browser/layout_test/layout_test_browser_main.cc:68:
      New code should not use ScopedAllowIO. Post a task to the blocking
      pool or the FILE thread instead.
    content/shell/browser/layout_test/layout_test_browser_main.cc:86:
      New code should not use ScopedAllowIO. Post a task to the blocking
      pool or the FILE thread instead.
    content/shell/browser/layout_test/layout_test_browser_main.cc:188:
      New code should not use ScopedAllowIO. Post a task to the blocking
      pool or the FILE thread instead.

[Mike West, 2014-10-16 12:08:18]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-10-16 12:08:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/649223004/60001

[commit-bot: I haz the power, 2014-10-16 12:09:07]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-10-16 12:09:56]

Patchset 4 (id:??) landed as
https://crrev.com/88e23180127784b736fa6b1d105d324add1d8151
Cr-Commit-Position: refs/heads/master@{#299879}

[marja, 2014-10-20 09:52:33]

On 2014/10/16 12:00:06, Mike West wrote:
> That said, the header bit does look weird: +marja. :)
> 
> ---
> 
> ** Presubmit Messages **
> Your #include order seems to be broken. Send mail to
> mailto:marja@chromium.org if this is not the case.
>       content/shell/browser/layout_test/layout_test_browser_main.cc:7 \
>       content/shell/browser/layout_test/layout_test_browser_main.cc:9

I think this is because #include "content/shell/browser/shell_browser_main.h" is
not recognized as a special first include, if the test file is
layout_test_browser_main.cc. That looks reasonable. Why not call this file
layout_test_shell_browser_main.cc, if it's testing shell_browser_main.h ? Or if
not, the include should be moved to where it belongs.

[Mike West, 2014-10-20 09:56:53]

On 2014/10/20 09:52:33, marja wrote:
> On 2014/10/16 12:00:06, Mike West wrote:
> > That said, the header bit does look weird: +marja. :)
> > 
> > ---
> > 
> > ** Presubmit Messages **
> > Your #include order seems to be broken. Send mail to
> > mailto:marja@chromium.org if this is not the case.
> >       content/shell/browser/layout_test/layout_test_browser_main.cc:7 \
> >       content/shell/browser/layout_test/layout_test_browser_main.cc:9
> 
> I think this is because #include "content/shell/browser/shell_browser_main.h"
is
> not recognized as a special first include, if the test file is
> layout_test_browser_main.cc. That looks reasonable. Why not call this file
> layout_test_shell_browser_main.cc, if it's testing shell_browser_main.h ? Or
if
> not, the include should be moved to where it belongs.

*facepalm* I completely missed that. How did this even compile?

https://codereview.chromium.org/660473003