Remove duplication between TestRunner's fields and LayoutDumpFlags struct.

Remove duplication between TestRunner's fields and LayoutDumpFlags struct.

The current CL paves the way for 1) moving more test flags from
TestRunner's fields into LayoutDumpFlags and 2) replicating the
test flags when transferring to another renderer process (in addition
to using LayoutDumpFlags during a textual layout dump).

In a way, the current CL backpedals on some aspects of an earlier CL
(crrev.com/1589643003) which has introduced LayoutDumpFlags struct that
holds layout dump flags for the purpose of passing them to the browser
process (which can coordinate dumping across all frames).  The
backpedaling is necessary because:

- Having LayoutDumpFlags is good for passing the flags to the browser
  process (when having to do a layout dump spanning multiple renderers /
  OOPIFs), but LayoutDumpFlags is also bad, because the same flags are
  represented (in a slightly different way) in 2 places: in
  LayoutDumpFlags struct and in TestRunner fields.  This CL fixes this
  by moving relevant fields of TestRunner into LayoutDumpFlags struct.

- The previous CL tried to be (unnecessarily :-/) smart and for example:
  - consolidated |dump_child_frames_as_text|,
    |dump_child_frames_as_markup| and |dump_child_frame_scroll_positions|
    into |dump_child_frames| field.
  - consolidated |dump_as_text|, |dump_as_markup| into a |main_dump_mode|
    field with 3 possible states - text / markup / scroll positions.
  The current CL backpedals on this and simply copies TestRunner's fields
  into LayoutDumpFlags, because otherwise preserving all existing test
  behavior is prooving to be really difficult.

BUG=587175
Committed: https://crrev.com/9b9d70ea6efc122e1957198be5e7bba8924efaad
Cr-Commit-Position: refs/heads/master@{#377712}

[Łukasz Anforowicz, 2016-02-16 19:38:29]

Description was changed from

==========
Remove redundant TestRunner's fields related to layout dump flags.

crrev.com/1589643003 has introduced LayoutDumpFlags struct that
holds layout dump flags for the purpose of passing them to the
browser process (which can coordinate dumping across all frames).
The current CL cleans-up the fields of TestRunner class, that
are already represented in LayoutDumpFlags.

BUG=477150
==========

to

==========
Remove duplication between TestRunner's fields and LayoutDumpFlags struct.

The current CL paves the way for 1) moving more test flags from
TestRunner's fields into LayoutDumpFlags and 2) replicating the
test flags when transferring to another renderer process (in addition
to using LayoutDumpFlags during a textual layout dump).

In a way, the current CL backpedals on some aspects of an earlier CL
(crrev.com/1589643003) which has introduced LayoutDumpFlags struct that
holds layout dump flags for the purpose of passing them to the browser
process (which can coordinate dumping across all frames).  The
backpedaling is necessary because:

- Having LayoutDumpFlags is good for passing the flags to the browser
  process (when having to do a layout dump spanning multiple renderers /
  OOPIFs), but LayoutDumpFlags is also bad, because the same flags are
  represented (in a slightly different way) in 2 places: in
  LayoutDumpFlags struct and in TestRunner fields.  This CL fixes this
  by moving relevant fields of TestRunner into LayoutDumpFlags struct.

- The previous CL tried to be (unnecessarily :-/) smart and for example:
  - consolidated |dump_child_frames_as_text|,
    |dump_child_frames_as_markup| and |dump_child_frame_scroll_positions|
    into |dump_child_frames| field.
  - consolidated |dump_as_text|, |dump_as_markup| into a |main_dump_mode|
    field with 3 possible states - text / markup / scroll positions.
  The current CL backpedals on this and simply copies TestRunner's fields
  into LayoutDumpFlags, because otherwise preserving all existing test
  behavior is prooving to be really difficult.

BUG=587175
==========

[Łukasz Anforowicz, 2016-02-16 23:42:59]

Description was changed from

==========
Remove duplication between TestRunner's fields and LayoutDumpFlags struct.

The current CL paves the way for 1) moving more test flags from
TestRunner's fields into LayoutDumpFlags and 2) replicating the
test flags when transferring to another renderer process (in addition
to using LayoutDumpFlags during a textual layout dump).

In a way, the current CL backpedals on some aspects of an earlier CL
(crrev.com/1589643003) which has introduced LayoutDumpFlags struct that
holds layout dump flags for the purpose of passing them to the browser
process (which can coordinate dumping across all frames).  The
backpedaling is necessary because:

- Having LayoutDumpFlags is good for passing the flags to the browser
  process (when having to do a layout dump spanning multiple renderers /
  OOPIFs), but LayoutDumpFlags is also bad, because the same flags are
  represented (in a slightly different way) in 2 places: in
  LayoutDumpFlags struct and in TestRunner fields.  This CL fixes this
  by moving relevant fields of TestRunner into LayoutDumpFlags struct.

- The previous CL tried to be (unnecessarily :-/) smart and for example:
  - consolidated |dump_child_frames_as_text|,
    |dump_child_frames_as_markup| and |dump_child_frame_scroll_positions|
    into |dump_child_frames| field.
  - consolidated |dump_as_text|, |dump_as_markup| into a |main_dump_mode|
    field with 3 possible states - text / markup / scroll positions.
  The current CL backpedals on this and simply copies TestRunner's fields
  into LayoutDumpFlags, because otherwise preserving all existing test
  behavior is prooving to be really difficult.

BUG=587175
==========

to

==========
Remove duplication between TestRunner's fields and LayoutDumpFlags struct.

The current CL paves the way for 1) moving more test flags from
TestRunner's fields into LayoutDumpFlags and 2) replicating the
test flags when transferring to another renderer process (in addition
to using LayoutDumpFlags during a textual layout dump).

In a way, the current CL backpedals on some aspects of an earlier CL
(crrev.com/1589643003) which has introduced LayoutDumpFlags struct that
holds layout dump flags for the purpose of passing them to the browser
process (which can coordinate dumping across all frames).  The
backpedaling is necessary because:

- Having LayoutDumpFlags is good for passing the flags to the browser
  process (when having to do a layout dump spanning multiple renderers /
  OOPIFs), but LayoutDumpFlags is also bad, because the same flags are
  represented (in a slightly different way) in 2 places: in
  LayoutDumpFlags struct and in TestRunner fields.  This CL fixes this
  by moving relevant fields of TestRunner into LayoutDumpFlags struct.

- The previous CL tried to be (unnecessarily :-/) smart and for example:
  - consolidated |dump_child_frames_as_text|,
    |dump_child_frames_as_markup| and |dump_child_frame_scroll_positions|
    into |dump_child_frames| field.
  - consolidated |dump_as_text|, |dump_as_markup| into a |main_dump_mode|
    field with 3 possible states - text / markup / scroll positions.
  The current CL backpedals on this and simply copies TestRunner's fields
  into LayoutDumpFlags, because otherwise preserving all existing test
  behavior is prooving to be really difficult.

BUG=587175
==========

[Łukasz Anforowicz, 2016-02-18 16:52:18]

lukasza@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[Łukasz Anforowicz, 2016-02-18 16:52:19]

Dominic, could you please take a look?

[Łukasz Anforowicz, 2016-02-18 18:23:51]

lukasza@chromium.org changed reviewers:
+ dcheng@chromium.org

[Łukasz Anforowicz, 2016-02-18 18:23:52]

Actually, Daniel - could you take a first stab at reviewing this?  You might
have a better overview of the context behind these CLs.

[Łukasz Anforowicz, 2016-02-22 18:34:15]

Daniel, I think that this CL is ready for a review - I think it makes sense to
land this clean-up, even if crrev.com/1701803003 might still need significant
rework and if crrev.com/1711853002 might need to be abandoned.

[dcheng, 2016-02-23 18:26:35]

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
File components/test_runner/layout_dump.cc (right):

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
components/test_runner/layout_dump.cc:75: } else {
Hmm... maybe we should still explicitly have a bool to track the dump scroll
positions state?

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
File components/test_runner/layout_dump_flags.h (right):

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
components/test_runner/layout_dump_flags.h:16: // dump all frames as plain text.
Hmm, so this implies that if I call dumpChildFramesAsText() but forget to call
dumpAsText(), the child frames just won't get dumped?

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
components/test_runner/layout_dump_flags.h:20: // representation of the
renderer.
s/renderer/layout objects/ might be more accurate

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
File components/test_runner/test_runner.cc (right):

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
components/test_runner/test_runner.cc:1853: CheckResponseMimeType();
Why is this line here?

[Łukasz Anforowicz, 2016-02-23 18:45:27]

Daniel, can you take another look please?

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
File components/test_runner/layout_dump.cc (right):

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
components/test_runner/layout_dump.cc:75: } else {
On 2016/02/23 18:26:35, dcheng wrote:
> Hmm... maybe we should still explicitly have a bool to track the dump scroll
> positions state?

I am trying to make this CL a mechanical translation of fields of TestRunner
class into fields of LayoutDumpFlags class.  TestRunner doesn't have a separate
field to express "dump scroll positions" mode - this mode is active when
|!dump_as_text && !dump_as_markup|.

Here and elsewhere I squirm and I agree that TestRunner's flags are a bit of a
mess.  We might want to clean up this mess, but I would want to do it in a
separate CL.  I tried to consolidate (as seen in the previous shape of
LayoutTestFlags), but there are so many gotchas that it is really difficult to
keep all the test passing during the consolidation / refactoring.

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
File components/test_runner/layout_dump_flags.h (right):

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
components/test_runner/layout_dump_flags.h:16: // dump all frames as plain text.
On 2016/02/23 18:26:35, dcheng wrote:
> Hmm, so this implies that if I call dumpChildFramesAsText() but forget to call
> dumpAsText(), the child frames just won't get dumped?

Correct :-(.

There are tests that do something like:
- dumpChildFramesScrollPositions()
- dumpAsText()
and expect to dump only "text" of the main frame.

And there are tests that call
- dumpChildFramesScrollPositions() from one place
- and rely on dumpAsText being called from a totally separate place.

Still - this CL tries to prepare a way for having all the flags in a single
place (for replication purposes).  Once the flags are divorced from unrelated
code in TestRunner, maybe we can attempt once again to consolidate them...

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
components/test_runner/layout_dump_flags.h:20: // representation of the
renderer.
On 2016/02/23 18:26:35, dcheng wrote:
> s/renderer/layout objects/ might be more accurate

Done.

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
File components/test_runner/test_runner.cc (right):

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
components/test_runner/test_runner.cc:1853: CheckResponseMimeType();
On 2016/02/23 18:26:35, dcheng wrote:
> Why is this line here?

Previously this call was made by shouldDumpAsText function (removed in this CL /
called by the old version of GetLayoutDumpFlags).

[Łukasz Anforowicz, 2016-02-23 18:50:40]

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
File components/test_runner/layout_dump_flags.h (right):

https://codereview.chromium.org/1689283003/diff/140001/components/test_runner...
components/test_runner/layout_dump_flags.h:16: // dump all frames as plain text.
On 2016/02/23 18:45:26, Łukasz Anforowicz wrote:
> On 2016/02/23 18:26:35, dcheng wrote:
> > Hmm, so this implies that if I call dumpChildFramesAsText() but forget to
call
> > dumpAsText(), the child frames just won't get dumped?
> 
> Correct :-(.
> 
> There are tests that do something like:
> - dumpChildFramesScrollPositions()
> - dumpAsText()
> and expect to dump only "text" of the main frame.
> 
> And there are tests that call
> - dumpChildFramesScrollPositions() from one place
> - and rely on dumpAsText being called from a totally separate place.

Correction - this was supposed to say:

And there are tests that call
- dumpChildFramesAsText() from one place
- and rely on dumpAsText() being called from a totally separate place (i.e. all
tests under http/tests/xmlhttprequest/origin-exact-matching).

[jochen (gone - plz use gerrit), 2016-02-23 18:52:52]

Similar to content/browser, content/shell/browser must only include pure POD and
enum headers from renderer components

Enum values should also define a _LAST value, and it would be nice if the struct
contained a ctor that initializes all members

[Łukasz Anforowicz, 2016-02-23 19:04:47]

On 2016/02/23 18:52:52, jochen wrote:
> Similar to content/browser, content/shell/browser must only include pure POD
and
> enum headers from renderer components

Right - I believe that LayoutDumpFlags is a pure POD (no user-defined default
constructor, assignment, copy constructor + fields are POD types themselves
[bools]).

> Enum values should also define a _LAST value,

I am removing the enum / only the struct is left after this CL.

> and it would be nice if the struct
> contained a ctor that initializes all members

The user-defined default ctor would make it a non-POD.  This statement is based
on 1) http://stackoverflow.com/a/7189821, and 2) my work on gpu::IdType<...>
which doesn't pass POD tests from <type_traits> because of the default
constructor.

[jochen (gone - plz use gerrit), 2016-02-24 09:56:31]

On 2016/02/23 at 19:04:47, lukasza wrote:
> On 2016/02/23 18:52:52, jochen wrote:
> > Similar to content/browser, content/shell/browser must only include pure POD
and
> > enum headers from renderer components
> 
> Right - I believe that LayoutDumpFlags is a pure POD (no user-defined default
constructor, assignment, copy constructor + fields are POD types themselves
[bools]).
> 
> > Enum values should also define a _LAST value,
> 
> I am removing the enum / only the struct is left after this CL.
> 
> > and it would be nice if the struct
> > contained a ctor that initializes all members
> 
> The user-defined default ctor would make it a non-POD.  This statement is
based on 1) http://stackoverflow.com/a/7189821, and 2) my work on
gpu::IdType<...> which doesn't pass POD tests from <type_traits> because of the
default constructor.

right, but I'm asking for a non-default ctor that takes parameters for all
members (which is allowed).

it would also be cool if you could modify the browser & common DEPS to only
allow this header.

[Łukasz Anforowicz, 2016-02-24 18:08:47]

Thanks jochen@ - can you take another look?

On 2016/02/24 09:56:31, jochen wrote:
> On 2016/02/23 at 19:04:47, lukasza wrote:
> > On 2016/02/23 18:52:52, jochen wrote:
> > > Similar to content/browser, content/shell/browser must only include pure
POD
> and
> > > enum headers from renderer components
> > 
> > Right - I believe that LayoutDumpFlags is a pure POD (no user-defined
default
> constructor, assignment, copy constructor + fields are POD types themselves
> [bools]).
> > 
> > > Enum values should also define a _LAST value,
> > 
> > I am removing the enum / only the struct is left after this CL.
> > 
> > > and it would be nice if the struct
> > > contained a ctor that initializes all members
> > 
> > The user-defined default ctor would make it a non-POD.  This statement is
> based on 1) http://stackoverflow.com/a/7189821, and 2) my work on
> gpu::IdType<...> which doesn't pass POD tests from <type_traits> because of
the
> default constructor.
> 
> right, but I'm asking for a non-default ctor that takes parameters for all
> members (which is allowed).

Ok - done.

Please note that I am working on removing the browser-side dependency on
LayoutDumpFlags in crrev.com/1715573002 (work-in-progress, not ready for a real
review or landing).  With this dependency removed, we will be able to provide a
user-defined default constructor for LayoutDumpFlags (and therefore remove the
somewhat yucky split where TestRunner controls initial/default values of
LayoutDumpFlags only because of history + because of POD requirements).

> it would also be cool if you could modify the browser & common DEPS to only
> allow this header.

Let me try to tackle this in a separate CL - crrev.com/1730303002.

Please note that right now (i.e. before this CL) content/shell/DEPS is already
singling out layout_dump_flags.h as a POD that is safe to depend on from the
browser side:

$ find content -name DEPS | xargs grep test_runner
...
content/shell/DEPS:  "+components/test_runner/layout_dump_flags.h",  # POD only.
...

[jochen (gone - plz use gerrit), 2016-02-24 18:27:55]

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

[jochen (gone - plz use gerrit), 2016-02-24 18:27:55]

lgtm

btw, I plan to move the entire test_runner component to
content/shell/renderer/test_runner (crbug.com/585292)

[Łukasz Anforowicz, 2016-02-24 18:34:27]

On 2016/02/24 18:27:55, jochen wrote:
> lgtm

Thanks for reviewing :-)

> btw, I plan to move the entire test_runner component to
> content/shell/renderer/test_runner (crbug.com/585292)

Yes - this is one reason why I am trying to flush out at least some of my
in-flight CLs touching this component...

I hope that at least the initial move will be more-or-less mechanical (i.e.
without any major changes of file contents) and therefore rebasing will be easy.

Are there any plans to remove extra layers of abstractions that used to be
needed by components/html_viewer (i.e. some of the abstract interfaces from
components/test_runner are now only implemented under content/shell/renderer -
in theory this extra layers of abstraction can be removed, but this seems like
quite a bit of work... :-/ ).

[Łukasz Anforowicz, 2016-02-24 18:36:01]

The CQ bit was checked by lukasza@chromium.org

[jochen (gone - plz use gerrit), 2016-02-24 18:37:24]

On 2016/02/24 at 18:34:27, lukasza wrote:
> On 2016/02/24 18:27:55, jochen wrote:
> > lgtm
> 
> Thanks for reviewing :-)
> 
> > btw, I plan to move the entire test_runner component to
> > content/shell/renderer/test_runner (crbug.com/585292)
> 
> Yes - this is one reason why I am trying to flush out at least some of my
in-flight CLs touching this component...
> 
> I hope that at least the initial move will be more-or-less mechanical (i.e.
without any major changes of file contents) and therefore rebasing will be easy.
> 
> Are there any plans to remove extra layers of abstractions that used to be
needed by components/html_viewer (i.e. some of the abstract interfaces from
components/test_runner are now only implemented under content/shell/renderer -
in theory this extra layers of abstraction can be removed, but this seems like
quite a bit of work... :-/ ).

well, I kinda like that abstraction - the test_runner component only depends on
blink which is nice.

[commit-bot: I haz the power, 2016-02-24 18:44:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1689283003/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1689283003/220001

[commit-bot: I haz the power, 2016-02-24 20:11:12]

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

[commit-bot: I haz the power, 2016-02-24 20:11:13]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Łukasz Anforowicz, 2016-02-25 18:16:10]

The patchset that got l-g-t-m moved setting of LayoutDumpFlags from
TestRunner::Reset into the constructor of TestRunner (I am not sure why I did
this - one reason was a desire to make sure that the LayoutDumpFlags constructor
suggested by jochen@ actually has at least one caller).  Looking at this move
today I see that it is wrong - TestRunner::Reset needs to reset the flags to the
defaults (the bots turned red on the last patchset because of this).

So - I've re-added setting of the flags in TestRunner::Reset in patchset #13
[1].  In theory I could have called here the LayoutDumpFlags constructor taking
all the field values, but I really prefer having the fields named by code rather
than by comments (+ doing it this way makes the diff highlight that it is a
mechanical move of fields from TestRunner into LayoutDumpFlags).  I think this
is ok, (and even if this is not perfect, I am not sure if it is worth spending
more time on this, given that I will be removing the LayoutDumpFlags(bool x 8)
constructor and replacing it with a default constructor soon - in
crrev.com/1715573002).

[1]
https://codereview.chromium.org/1689283003/diff2/220001:240001/components/tes...

[Łukasz Anforowicz, 2016-02-25 22:16:08]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-02-25 22:16:08]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1689283003/#ps240001
(title: "Despite LayoutDumpFlags constructor, we still need to set the flags in
TestRunner::Reset.")

[commit-bot: I haz the power, 2016-02-25 22:25:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1689283003/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1689283003/240001

[commit-bot: I haz the power, 2016-02-25 23:45:51]

Description was changed from

==========
Remove duplication between TestRunner's fields and LayoutDumpFlags struct.

The current CL paves the way for 1) moving more test flags from
TestRunner's fields into LayoutDumpFlags and 2) replicating the
test flags when transferring to another renderer process (in addition
to using LayoutDumpFlags during a textual layout dump).

In a way, the current CL backpedals on some aspects of an earlier CL
(crrev.com/1589643003) which has introduced LayoutDumpFlags struct that
holds layout dump flags for the purpose of passing them to the browser
process (which can coordinate dumping across all frames).  The
backpedaling is necessary because:

- Having LayoutDumpFlags is good for passing the flags to the browser
  process (when having to do a layout dump spanning multiple renderers /
  OOPIFs), but LayoutDumpFlags is also bad, because the same flags are
  represented (in a slightly different way) in 2 places: in
  LayoutDumpFlags struct and in TestRunner fields.  This CL fixes this
  by moving relevant fields of TestRunner into LayoutDumpFlags struct.

- The previous CL tried to be (unnecessarily :-/) smart and for example:
  - consolidated |dump_child_frames_as_text|,
    |dump_child_frames_as_markup| and |dump_child_frame_scroll_positions|
    into |dump_child_frames| field.
  - consolidated |dump_as_text|, |dump_as_markup| into a |main_dump_mode|
    field with 3 possible states - text / markup / scroll positions.
  The current CL backpedals on this and simply copies TestRunner's fields
  into LayoutDumpFlags, because otherwise preserving all existing test
  behavior is prooving to be really difficult.

BUG=587175
==========

to

==========
Remove duplication between TestRunner's fields and LayoutDumpFlags struct.

The current CL paves the way for 1) moving more test flags from
TestRunner's fields into LayoutDumpFlags and 2) replicating the
test flags when transferring to another renderer process (in addition
to using LayoutDumpFlags during a textual layout dump).

In a way, the current CL backpedals on some aspects of an earlier CL
(crrev.com/1589643003) which has introduced LayoutDumpFlags struct that
holds layout dump flags for the purpose of passing them to the browser
process (which can coordinate dumping across all frames).  The
backpedaling is necessary because:

- Having LayoutDumpFlags is good for passing the flags to the browser
  process (when having to do a layout dump spanning multiple renderers /
  OOPIFs), but LayoutDumpFlags is also bad, because the same flags are
  represented (in a slightly different way) in 2 places: in
  LayoutDumpFlags struct and in TestRunner fields.  This CL fixes this
  by moving relevant fields of TestRunner into LayoutDumpFlags struct.

- The previous CL tried to be (unnecessarily :-/) smart and for example:
  - consolidated |dump_child_frames_as_text|,
    |dump_child_frames_as_markup| and |dump_child_frame_scroll_positions|
    into |dump_child_frames| field.
  - consolidated |dump_as_text|, |dump_as_markup| into a |main_dump_mode|
    field with 3 possible states - text / markup / scroll positions.
  The current CL backpedals on this and simply copies TestRunner's fields
  into LayoutDumpFlags, because otherwise preserving all existing test
  behavior is prooving to be really difficult.

BUG=587175
==========

[commit-bot: I haz the power, 2016-02-25 23:45:52]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2016-02-25 23:47:36]

Description was changed from

==========
Remove duplication between TestRunner's fields and LayoutDumpFlags struct.

The current CL paves the way for 1) moving more test flags from
TestRunner's fields into LayoutDumpFlags and 2) replicating the
test flags when transferring to another renderer process (in addition
to using LayoutDumpFlags during a textual layout dump).

In a way, the current CL backpedals on some aspects of an earlier CL
(crrev.com/1589643003) which has introduced LayoutDumpFlags struct that
holds layout dump flags for the purpose of passing them to the browser
process (which can coordinate dumping across all frames).  The
backpedaling is necessary because:

- Having LayoutDumpFlags is good for passing the flags to the browser
  process (when having to do a layout dump spanning multiple renderers /
  OOPIFs), but LayoutDumpFlags is also bad, because the same flags are
  represented (in a slightly different way) in 2 places: in
  LayoutDumpFlags struct and in TestRunner fields.  This CL fixes this
  by moving relevant fields of TestRunner into LayoutDumpFlags struct.

- The previous CL tried to be (unnecessarily :-/) smart and for example:
  - consolidated |dump_child_frames_as_text|,
    |dump_child_frames_as_markup| and |dump_child_frame_scroll_positions|
    into |dump_child_frames| field.
  - consolidated |dump_as_text|, |dump_as_markup| into a |main_dump_mode|
    field with 3 possible states - text / markup / scroll positions.
  The current CL backpedals on this and simply copies TestRunner's fields
  into LayoutDumpFlags, because otherwise preserving all existing test
  behavior is prooving to be really difficult.

BUG=587175
==========

to

==========
Remove duplication between TestRunner's fields and LayoutDumpFlags struct.

The current CL paves the way for 1) moving more test flags from
TestRunner's fields into LayoutDumpFlags and 2) replicating the
test flags when transferring to another renderer process (in addition
to using LayoutDumpFlags during a textual layout dump).

In a way, the current CL backpedals on some aspects of an earlier CL
(crrev.com/1589643003) which has introduced LayoutDumpFlags struct that
holds layout dump flags for the purpose of passing them to the browser
process (which can coordinate dumping across all frames).  The
backpedaling is necessary because:

- Having LayoutDumpFlags is good for passing the flags to the browser
  process (when having to do a layout dump spanning multiple renderers /
  OOPIFs), but LayoutDumpFlags is also bad, because the same flags are
  represented (in a slightly different way) in 2 places: in
  LayoutDumpFlags struct and in TestRunner fields.  This CL fixes this
  by moving relevant fields of TestRunner into LayoutDumpFlags struct.

- The previous CL tried to be (unnecessarily :-/) smart and for example:
  - consolidated |dump_child_frames_as_text|,
    |dump_child_frames_as_markup| and |dump_child_frame_scroll_positions|
    into |dump_child_frames| field.
  - consolidated |dump_as_text|, |dump_as_markup| into a |main_dump_mode|
    field with 3 possible states - text / markup / scroll positions.
  The current CL backpedals on this and simply copies TestRunner's fields
  into LayoutDumpFlags, because otherwise preserving all existing test
  behavior is prooving to be really difficult.

BUG=587175

Committed: https://crrev.com/9b9d70ea6efc122e1957198be5e7bba8924efaad
Cr-Commit-Position: refs/heads/master@{#377712}
==========

[commit-bot: I haz the power, 2016-02-25 23:47:38]

Patchset 13 (id:??) landed as
https://crrev.com/9b9d70ea6efc122e1957198be5e7bba8924efaad
Cr-Commit-Position: refs/heads/master@{#377712}