Use NGLogicalRect instead of NGExclusion for exclusions.

Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where the writing mode is inored

BUG=635619
Committed: https://crrev.com/250a6979a3311cb8a39ca7d7f316353550362f86
Cr-Commit-Position: refs/heads/master@{#428907}

[Gleb Lanbin, 2016-10-28 20:26:31]

Patchset #1 (id:1) has been deleted

[Gleb Lanbin, 2016-10-28 20:26:58]

glebl@chromium.org changed reviewers:
+ cbiesinger@chromium.org, ikilpatrick@chromium.org

[Gleb Lanbin, 2016-10-28 20:27:18]

Description was changed from

==========
Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through
Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where
the writing mode is inored
- Introduce NGLogicalRect::Builder to improve readability.
- Add a copy of ptr_util.h

BUG=635619
==========

to

==========
Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through
Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where
the writing mode is inored
- Introduce NGLogicalRect::Builder to improve readability.
- Add a copy of ptr_util.h

BUG=635619
==========

[Gleb Lanbin, 2016-10-28 20:42:03]

The CQ bit was checked by glebl@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 20:42:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 20:51:41]

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

[commit-bot: I haz the power, 2016-10-28 20:51:42]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Gleb Lanbin, 2016-10-28 20:55:46]

Patchset #1 (id:20001) has been deleted

[Gleb Lanbin, 2016-10-28 20:55:59]

The CQ bit was checked by glebl@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 20:56:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Gleb Lanbin, 2016-10-28 21:38:04]

Patchset #1 (id:40001) has been deleted

[Gleb Lanbin, 2016-10-28 21:38:12]

The CQ bit was checked by glebl@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 21:38:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Gleb Lanbin, 2016-10-28 23:05:16]

On 2016/10/28 20:26:59, glebl wrote:

per Ian's suggestion, layout/ng/ptr_util.h will be moved to WTF/PtrUtil.h
http://crrev.com/2455313003

[ikilpatrick, 2016-10-28 23:18:23]

ikilpatrick@chromium.org changed reviewers:
+ eae@chromium.org

[ikilpatrick, 2016-10-28 23:18:25]

looks good! would like others to have a look and see what they think of the
builder pattern.

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc (right):

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc:88: "space.");
previous line?

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h (right):

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h:63: "space.");
can fit on previous line?

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_units.h (right):

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_units.h:130: class Builder {
yeah, i'm not sold on using the builder for these types. but would like others
opinions :)

[Gleb Lanbin, 2016-10-28 23:54:09]

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc (right):

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc:88: "space.");
On 2016/10/28 23:18:24, ikilpatrick wrote:
> previous line?

Done.

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h (right):

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h:63: "space.");
On 2016/10/28 23:18:24, ikilpatrick wrote:
> can fit on previous line?

Done.

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_units.h (right):

https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_units.h:130: class Builder {
On 2016/10/28 23:18:24, ikilpatrick wrote:
> yeah, i'm not sold on using the builder for these types. but would like others
> opinions :)

yes, I'm not a 100% fan of it either because it's too verbose. That's why I
added a TODO to move it in a separate file. In google3 this boilerplate code for
builders would be pre-generated.

just want to list other variants that were considered.
1) Use constructor.
Example:
new NGExclusion(LayoutUnit(0), LayoutUnit(600), LayoutUnit(100),
LayoutUnit(500));
Cons: impossible to read and error-prone. It's not clear what is set to what.
Pros: concise and already available

2) Designated Initializers
Example: {.inline_offset = LayoutUnit(50), .block_offset = LayoutUnit(50)} etc.
Cons: easy to read, concise and already available
Pros: it's a part of C99 standard. Our MSVC 2015 doesn't support it
unfortunately.

3) Comments
Example: {/* inline_offset */ LayoutUnit(50), /* block_offset */LayoutUnit(50)}
etc.
Cons: easy to read, concise and already available
Pros: it's hard to enforce this rule if it's not a part of API or style rules.

4) Builders + Comments.
For small structs lets use comments. For big structs lets use builders. 
NGLogicalOffset, NGLogicalSize should also have builders

in this case we can split this builder.
Example:
NGLogicalRect::Builder()
    .SetOffset(
        NGLogicalOffset::Builder()
        .SetInlineOffset(bla)
        .SetBlocOffset(bla).Build())
    .SetSize(
        NGLogicalSize::Builder()
        .SetInlineSize(bla)
        .SetBlockSize(bla).Build())
    .Build()

[Gleb Lanbin, 2016-10-28 23:58:36]

On 2016/10/28 23:54:09, glebl wrote:
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc (right):
> 
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc:88: "space.");
> On 2016/10/28 23:18:24, ikilpatrick wrote:
> > previous line?
> 
> Done.
> 
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h (right):
> 
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h:63: "space.");
> On 2016/10/28 23:18:24, ikilpatrick wrote:
> > can fit on previous line?
> 
> Done.
> 
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/ng/ng_units.h (right):
> 
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/ng/ng_units.h:130: class Builder {
> On 2016/10/28 23:18:24, ikilpatrick wrote:
> > yeah, i'm not sold on using the builder for these types. but would like
others
> > opinions :)
> 
> yes, I'm not a 100% fan of it either because it's too verbose. That's why I
> added a TODO to move it in a separate file. In google3 this boilerplate code
for
> builders would be pre-generated.
> 
> just want to list other variants that were considered.
> 1) Use constructor.
> Example:
> new NGExclusion(LayoutUnit(0), LayoutUnit(600), LayoutUnit(100),
> LayoutUnit(500));
> Cons: impossible to read and error-prone. It's not clear what is set to what.
> Pros: concise and already available
> 
> 2) Designated Initializers
> Example: {.inline_offset = LayoutUnit(50), .block_offset = LayoutUnit(50)}
etc.
> Cons: easy to read, concise and already available
> Pros: it's a part of C99 standard. Our MSVC 2015 doesn't support it
> unfortunately.
> 
> 3) Comments
> Example: {/* inline_offset */ LayoutUnit(50), /* block_offset
*/LayoutUnit(50)}
> etc.
> Cons: easy to read, concise and already available
> Pros: it's hard to enforce this rule if it's not a part of API or style rules.
> 
> 4) Builders + Comments.
> For small structs lets use comments. For big structs lets use builders. 
> NGLogicalOffset, NGLogicalSize should also have builders
> 
> in this case we can split this builder.
> Example:
> NGLogicalRect::Builder()
>     .SetOffset(
>         NGLogicalOffset::Builder()
>         .SetInlineOffset(bla)
>         .SetBlocOffset(bla).Build())
>     .SetSize(
>         NGLogicalSize::Builder()
>         .SetInlineSize(bla)
>         .SetBlockSize(bla).Build())
>     .Build()

another option, ban all struct's constructors that have numbers of parameters >
1.

[Gleb Lanbin, 2016-10-31 20:55:32]

Description was changed from

==========
Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through
Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where
the writing mode is inored
- Introduce NGLogicalRect::Builder to improve readability.
- Add a copy of ptr_util.h

BUG=635619
==========

to

==========
Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through
Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where
the writing mode is inored
- Add a copy of ptr_util.h

BUG=635619
==========

[Gleb Lanbin, 2016-10-31 20:57:15]

On 2016/10/28 23:58:36, glebl wrote:
> On 2016/10/28 23:54:09, glebl wrote:
> >
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc
(right):
> > 
> >
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc:88:
"space.");
> > On 2016/10/28 23:18:24, ikilpatrick wrote:
> > > previous line?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h (right):
> > 
> >
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h:63:
"space.");
> > On 2016/10/28 23:18:24, ikilpatrick wrote:
> > > can fit on previous line?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/layout/ng/ng_units.h (right):
> > 
> >
>
https://codereview.chromium.org/2457013004/diff/60001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/layout/ng/ng_units.h:130: class Builder {
> > On 2016/10/28 23:18:24, ikilpatrick wrote:
> > > yeah, i'm not sold on using the builder for these types. but would like
> others
> > > opinions :)
> > 
> > yes, I'm not a 100% fan of it either because it's too verbose. That's why I
> > added a TODO to move it in a separate file. In google3 this boilerplate code
> for
> > builders would be pre-generated.
> > 
> > just want to list other variants that were considered.
> > 1) Use constructor.
> > Example:
> > new NGExclusion(LayoutUnit(0), LayoutUnit(600), LayoutUnit(100),
> > LayoutUnit(500));
> > Cons: impossible to read and error-prone. It's not clear what is set to
what.
> > Pros: concise and already available
> > 
> > 2) Designated Initializers
> > Example: {.inline_offset = LayoutUnit(50), .block_offset = LayoutUnit(50)}
> etc.
> > Cons: easy to read, concise and already available
> > Pros: it's a part of C99 standard. Our MSVC 2015 doesn't support it
> > unfortunately.
> > 
> > 3) Comments
> > Example: {/* inline_offset */ LayoutUnit(50), /* block_offset
> */LayoutUnit(50)}
> > etc.
> > Cons: easy to read, concise and already available
> > Pros: it's hard to enforce this rule if it's not a part of API or style
rules.
> > 
> > 4) Builders + Comments.
> > For small structs lets use comments. For big structs lets use builders. 
> > NGLogicalOffset, NGLogicalSize should also have builders
> > 
> > in this case we can split this builder.
> > Example:
> > NGLogicalRect::Builder()
> >     .SetOffset(
> >         NGLogicalOffset::Builder()
> >         .SetInlineOffset(bla)
> >         .SetBlocOffset(bla).Build())
> >     .SetSize(
> >         NGLogicalSize::Builder()
> >         .SetInlineSize(bla)
> >         .SetBlockSize(bla).Build())
> >     .Build()
> 
> another option, ban all struct's constructors that have numbers of parameters
>
> 1.

after talking to Ian and Christian offline I decided to remove
NGLogicalRect::Builder to speed up the review.

[Gleb Lanbin, 2016-10-31 21:35:02]

The CQ bit was checked by glebl@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-31 21:35:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ikilpatrick, 2016-10-31 21:36:08]

lgtm

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ng_physical_constraint_space.cc
(right):

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_physical_constraint_space.cc:50:
exclusions_.append(MakeUnique<const NGLogicalRect>(exclusion));
util the wtf ptr_util goes through can we just to a wrapUnique(new blah?)?

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ptr_util.h (right):

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ptr_util.h:5: #ifndef ptr_util_h
would prefer to wait until we have this in wtf instead.

[commit-bot: I haz the power, 2016-10-31 21:39:51]

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

[commit-bot: I haz the power, 2016-10-31 21:39:51]

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

[Gleb Lanbin, 2016-10-31 23:08:18]

thanks for the review

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ng_physical_constraint_space.cc
(right):

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_physical_constraint_space.cc:50:
exclusions_.append(MakeUnique<const NGLogicalRect>(exclusion));
On 2016/10/31 21:36:08, ikilpatrick wrote:
> util the wtf ptr_util goes through can we just to a wrapUnique(new blah?)?

Done.

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ptr_util.h (right):

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ptr_util.h:5: #ifndef ptr_util_h
On 2016/10/31 21:36:08, ikilpatrick wrote:
> would prefer to wait until we have this in wtf instead.

deleted

[atotic, 2016-10-31 23:42:41]

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ng_units.h (right):

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_units.h:107: bool IsWithin(const
NGLogicalRect& other) const;
Confusing to me. Does this mean this is fully enclosed inside other?

Will {0,0,10,10}.isWithin(0,0,10,10) return true?

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_units.h:107: bool IsWithin(const
NGLogicalRect& other) const;
IsWithin had me confused. 

Usual Rect APIs use 'contains' & 'intersect' (see Java, MS).

I suggest renaming it to IsContained()

https://codereview.chromium.org/2457013004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_units.h:116: return
offset.inline_offset + size.inline_size;
Only simple accessors should be inline according to style guide:

https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

https://codereview.chromium.org/2457013004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ng_units.h (right):

https://codereview.chromium.org/2457013004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_units.h:107: bool IsWithin(const
NGLogicalRect& other) const;
I was confused by "IsWithin".

Common rect APIs (Java, MS) use "contains" and "intersect"

I suggest renaming to IsContained.

https://codereview.chromium.org/2457013004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_units.h:116: return
offset.inline_offset + size.inline_size;
Chromium guidelines suggest:
"Simple accessors should generally be the only inline functions"

https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

[Gleb Lanbin, 2016-11-01 00:00:43]

https://codereview.chromium.org/2457013004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ng_units.h (right):

https://codereview.chromium.org/2457013004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_units.h:107: bool IsWithin(const
NGLogicalRect& other) const;
On 2016/10/31 23:42:40, atotic2 wrote:
> I was confused by "IsWithin".
> 
> Common rect APIs (Java, MS) use "contains" and "intersect"
> 
> I suggest renaming to IsContained.

renamed to IsContained. I'm OK with IsContained, IsInBounds or IsWithin.

https://codereview.chromium.org/2457013004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_units.h:116: return
offset.inline_offset + size.inline_size;
On 2016/10/31 23:42:40, atotic2 wrote:
> Chromium guidelines suggest:
> "Simple accessors should generally be the only inline functions"
> 
>
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

definitions inside class declarations are implicitly requested to be inlined.

[Gleb Lanbin, 2016-11-01 00:04:32]

glebl@chromium.org changed reviewers:
+ dpranke@gmail.com

[Gleb Lanbin, 2016-11-01 00:04:33]

adding dpranke@ to review BUILD.gn

[Gleb Lanbin, 2016-11-01 00:04:51]

glebl@chromium.org changed reviewers:
+ dpranke@chromium.org
- dpranke@gmail.com

[Gleb Lanbin, 2016-11-01 00:05:47]

Description was changed from

==========
Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through
Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where
the writing mode is inored
- Add a copy of ptr_util.h

BUG=635619
==========

to

==========
Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through
Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where
the writing mode is inored

BUG=635619
==========

[Gleb Lanbin, 2016-11-01 00:05:54]

The CQ bit was checked by glebl@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 00:06:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dirk Pranke, 2016-11-01 00:06:56]

BUILD.gn change lgtm.

[commit-bot: I haz the power, 2016-11-01 01:32:57]

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

[commit-bot: I haz the power, 2016-11-01 01:32:58]

Dry run: This issue passed the CQ dry run.

[Gleb Lanbin, 2016-11-01 01:33:34]

The CQ bit was checked by glebl@chromium.org

[Gleb Lanbin, 2016-11-01 01:33:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
ikilpatrick@chromium.org
Link to the patchset: https://codereview.chromium.org/2457013004/#ps110016
(title: "renamed IsWithing to IsContained")

[commit-bot: I haz the power, 2016-11-01 01:33:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 01:38:56]

Description was changed from

==========
Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through
Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where
the writing mode is inored

BUG=635619
==========

to

==========
Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through
Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where
the writing mode is inored

BUG=635619
==========

[commit-bot: I haz the power, 2016-11-01 01:38:57]

Committed patchset #5 (id:110016)

[commit-bot: I haz the power, 2016-11-01 01:42:18]

Description was changed from

==========
Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through
Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where
the writing mode is inored

BUG=635619
==========

to

==========
Use NGLogicalRect instead of NGExclusion to work with exclusions.

This patch does the following:
- Delete NGExclusion
- Change the code to use NGLogicalRect to store exclusions
- Make NGPhysicalConstraintSpace to own the list of exclusions through
Vector<std::unique_ptr>
- Introduce WRITING_MODE_IGNORED macros that can be used to annotate cases where
the writing mode is inored

BUG=635619

Committed: https://crrev.com/250a6979a3311cb8a39ca7d7f316353550362f86
Cr-Commit-Position: refs/heads/master@{#428907}
==========

[commit-bot: I haz the power, 2016-11-01 01:42:20]

Patchset 5 (id:??) landed as
https://crrev.com/250a6979a3311cb8a39ca7d7f316353550362f86
Cr-Commit-Position: refs/heads/master@{#428907}

[kjellander_chromium, 2016-11-01 10:29:54]

A revert of this CL (patchset #5 id:110016) has been created in
https://codereview.chromium.org/2467803002/ by kjellander@chromium.org.

The reason for reverting is: Reverting to enable reverting of
https://codereview.chromium.org/2458253002/ in
https://codereview.chromium.org/2458253002/ due to  merge conflicts.

Please reland once the flakiness has been sorted out..

[cbiesinger, 2016-11-01 15:02:01]

https://codereview.chromium.org/2457013004/diff/110016/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ng_fragment.cc (left):

https://codereview.chromium.org/2457013004/diff/110016/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_fragment.cc:13: // Changing the
writing mode establishes a new formatting context, for which
You deleted this line, and now the description doesn't make sense anymore...

[Gleb Lanbin, 2016-11-01 17:08:15]

On 2016/11/01 15:02:01, cbiesinger wrote:
>
https://codereview.chromium.org/2457013004/diff/110016/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/layout/ng/ng_fragment.cc (left):
> 
>
https://codereview.chromium.org/2457013004/diff/110016/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/layout/ng/ng_fragment.cc:13: // Changing the
> writing mode establishes a new formatting context, for which
> You deleted this line, and now the description doesn't make sense anymore...

thanks. here is the patch http://crrev.com/2470833002 to fix that.