Dirty line boxes correctly when a float gets layout

Dirty line boxes correctly when a float gets layout

https://codereview.chromium.org/1308433003 was still not
right. Revert to using dirtyLinesFromChangedChild() but
confine it to searching for lineboxes within the object.

BUG=616167
Committed: https://crrev.com/5db9460f869b506fd00f88ce74d60bd23d5f761a
Cr-Commit-Position: refs/heads/master@{#400248}

[rhogan, 2016-06-09 19:46:01]

robhogan@gmail.com changed reviewers:
+ cbiesinger@chromium.org, eae@chromium.org

[cbiesinger, 2016-06-09 23:03:36]

eae, I'll leave this review to you as you know line layout better, but I can
review it if you want me to

[eae, 2016-06-10 06:20:41]

https://codereview.chromium.org/2046863006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp (right):

https://codereview.chromium.org/2046863006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:1598:
setAncestorLineBoxDirty(ancestorLineBoxWasDirty);
Shouldn't this be setAncestorLineBoxDirty(ancestorLineBoxWasDirty ||
ancestorLineBoxDirty())?

[rhogan, 2016-06-10 17:25:13]

https://codereview.chromium.org/2046863006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp (right):

https://codereview.chromium.org/2046863006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:1598:
setAncestorLineBoxDirty(ancestorLineBoxWasDirty);
On 2016/06/10 at 06:20:41, eae wrote:
> Shouldn't this be setAncestorLineBoxDirty(ancestorLineBoxWasDirty ||
ancestorLineBoxDirty())?

Oh I have it the wrong way round. I need to set it to true before calling
dirtyLinesFromChangedChild, as I want to avoid climbing up the trees.

But when I reset it after calling dirtyLinesFromChangedChild() I do want to set
it back to whatever it was.

Can you take another look at the updated patch?

[rhogan, 2016-06-14 19:45:00]

https://codereview.chromium.org/2046863006/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/line/LineBoxList.h (right):

https://codereview.chromium.org/2046863006/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/line/LineBoxList.h:79: void
dirtyLinesFromChangedChild(LineLayoutItem parent, LineLayoutItem child, bool
canDirtyAncestors);
I used a bool here to avoid the need for including LayoutObject.h

[rhogan, 2016-06-15 19:04:29]

On 2016/06/14 at 19:45:00, rhogan wrote:
>
https://codereview.chromium.org/2046863006/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/line/LineBoxList.h (right):
> 
>
https://codereview.chromium.org/2046863006/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/line/LineBoxList.h:79: void
dirtyLinesFromChangedChild(LineLayoutItem parent, LineLayoutItem child, bool
canDirtyAncestors);
> I used a bool here to avoid the need for including LayoutObject.h

eae: got time to take a look?

[eae, 2016-06-16 15:03:27]

Thanks! This looks correct, thanks for the clarification,

LGTM

[eae, 2016-06-16 15:03:33]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2016-06-16 15:03:49]

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

[commit-bot: I haz the power, 2016-06-16 15:11:05]

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

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

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

[rhogan, 2016-06-16 18:00:37]

The CQ bit was checked by robhogan@gmail.com

[rhogan, 2016-06-16 18:00:38]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2046863006/#ps80001
(title: "Updated")

[commit-bot: I haz the power, 2016-06-16 18:01:43]

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

[commit-bot: I haz the power, 2016-06-16 20:21:48]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-16 20:24:01]

Description was changed from

==========
Dirty line boxes correctly when a float gets layout

https://codereview.chromium.org/1308433003 was still not
right. Revert to using dirtyLinesFromChangedChild() but
confine it to searching for lineboxes within the object.

BUG=616167
==========

to

==========
Dirty line boxes correctly when a float gets layout

https://codereview.chromium.org/1308433003 was still not
right. Revert to using dirtyLinesFromChangedChild() but
confine it to searching for lineboxes within the object.

BUG=616167

Committed: https://crrev.com/5db9460f869b506fd00f88ce74d60bd23d5f761a
Cr-Commit-Position: refs/heads/master@{#400248}
==========

[commit-bot: I haz the power, 2016-06-16 20:24:02]

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