Add NGLogicalOrigin and NGLogicalLeader parameters to NGLayoutOpportunityIterator.

Add NGLogicalOrigin and NGLogicalLeader parameters to NGLayoutOpportunityIterator.

NGLogicalOrigin will be used as a starting point for all layout opportunities generated by NGLayoutOpportunityIterator.
NGLogicalLeader will be used as a ending point of temporary excluded rectangle that represents a text fragment.

BUG=635619
Committed: https://crrev.com/6c72d27c88f4b2a17321e71951663a0b12c36d0d
Cr-Commit-Position: refs/heads/master@{#427580}

[Gleb Lanbin, 2016-10-25 17:08:57]

Patchset #1 (id:1) has been deleted

[Gleb Lanbin, 2016-10-25 17:09:47]

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

[cbiesinger, 2016-10-25 20:03:22]

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

[cbiesinger, 2016-10-25 20:03:23]

https://codereview.chromium.org/2451553005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h (left):

https://codereview.chromium.org/2451553005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h:121: bool
for_inline_or_bfc = false);
Why remove these two parameters -- what do you expect them to be replaced with?

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

https://codereview.chromium.org/2451553005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_units.h:54: // Origin(in logical
coordinates) for a fragment or inline content.
Space before ( please, in both comments.

[Gleb Lanbin, 2016-10-25 20:41:04]

PTAL

https://codereview.chromium.org/2451553005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h (left):

https://codereview.chromium.org/2451553005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h:121: bool
for_inline_or_bfc = false);
On 2016/10/25 20:03:23, cbiesinger wrote:
> Why remove these two parameters -- what do you expect them to be replaced
with?

the idea is to use NGLogicalOrigin instead, we will keep track of 2 last
NGLogicalOrigin for left and right floats. I discussed this with Ian and 
heagreed that this can be removed. It's not used anywhere.

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

https://codereview.chromium.org/2451553005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_units.h:54: // Origin(in logical
coordinates) for a fragment or inline content.
On 2016/10/25 20:03:23, cbiesinger wrote:
> Space before ( please, in both comments.

Done.

[cbiesinger, 2016-10-25 20:45:02]

On 2016/10/25 20:41:04, glebl wrote:
> PTAL
> 
>
https://codereview.chromium.org/2451553005/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h (left):
> 
>
https://codereview.chromium.org/2451553005/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h:121: bool
> for_inline_or_bfc = false);
> On 2016/10/25 20:03:23, cbiesinger wrote:
> > Why remove these two parameters -- what do you expect them to be replaced
> with?
> 
> the idea is to use NGLogicalOrigin instead, we will keep track of 2 last
> NGLogicalOrigin for left and right floats. I discussed this with Ian and 
> heagreed that this can be removed. It's not used anywhere.

It is *not yet* used. We will use this for inline and bfc children later, no?

Is the intent that the layout code will basically store last_float_edge_ =
std::max(last_float_edge_, current_float.MarginBoxInlineSize()) and pass that as
the origin for an inline/bfc? (Or, alternatively, maybe it's regular blocks that
need the special treatment?)

How will you handle clear?

[ikilpatrick, 2016-10-25 20:46:08]

API lgtm

[Gleb Lanbin, 2016-10-25 21:20:13]

On 2016/10/25 20:45:02, cbiesinger wrote:
> On 2016/10/25 20:41:04, glebl wrote:
> > PTAL
> > 
> >
>
https://codereview.chromium.org/2451553005/diff/20001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h (left):
> > 
> >
>
https://codereview.chromium.org/2451553005/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h:121: bool
> > for_inline_or_bfc = false);
> > On 2016/10/25 20:03:23, cbiesinger wrote:
> > > Why remove these two parameters -- what do you expect them to be replaced
> > with?
> > 
> > the idea is to use NGLogicalOrigin instead, we will keep track of 2 last
> > NGLogicalOrigin for left and right floats. I discussed this with Ian and 
> > heagreed that this can be removed. It's not used anywhere.
> 
> It is *not yet* used. We will use this for inline and bfc children later, no?
> 
> Is the intent that the layout code will basically store last_float_edge_ =
> std::max(last_float_edge_, current_float.MarginBoxInlineSize()) and pass that
as
> the origin for an inline/bfc? (Or, alternatively, maybe it's regular blocks
that
> need the special treatment?)
> 
> How will you handle clear?

lets have this discussion in the patch that actually implements 'clear' support.

the reason of this patch is to agree on API for NGLayoutOpportunityIterator.

I revert changed in NGConstraintSpace::LayoutOpportunities.

[Gleb Lanbin, 2016-10-25 21:20:52]

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

[commit-bot: I haz the power, 2016-10-25 21:21:43]

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-25 21:33:12]

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

[commit-bot: I haz the power, 2016-10-25 21:33:13]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  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_...)

[cbiesinger, 2016-10-25 21:43:47]

lgtm

[Gleb Lanbin, 2016-10-25 22:02:05]

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

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

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-25 23:32:50]

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

[commit-bot: I haz the power, 2016-10-25 23:32:51]

Dry run: This issue passed the CQ dry run.

[Gleb Lanbin, 2016-10-26 02:18:19]

The CQ bit was checked by glebl@chromium.org

[Gleb Lanbin, 2016-10-26 02:18:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
ikilpatrick@chromium.org, cbiesinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2451553005/#ps80001
(title: "do not redefine default LayoutOpportunities parameters")

[commit-bot: I haz the power, 2016-10-26 02:18:53]

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-26 02:32:45]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-10-26 02:34:43]

Description was changed from

==========
Add NGLogicalOrigin and NGLogicalLeader parameters to
NGLayoutOpportunityIterator.

NGLogicalOrigin will be used as a starting point for all layout opportunities
generated by NGLayoutOpportunityIterator.
NGLogicalLeader will be used as a ending point of temporary excluded rectangle
that represents a text fragment.

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

to

==========
Add NGLogicalOrigin and NGLogicalLeader parameters to
NGLayoutOpportunityIterator.

NGLogicalOrigin will be used as a starting point for all layout opportunities
generated by NGLayoutOpportunityIterator.
NGLogicalLeader will be used as a ending point of temporary excluded rectangle
that represents a text fragment.

BUG=635619

Committed: https://crrev.com/6c72d27c88f4b2a17321e71951663a0b12c36d0d
Cr-Commit-Position: refs/heads/master@{#427580}
==========

[commit-bot: I haz the power, 2016-10-26 02:34:44]

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