Handle nested position:sticky elements correctly (main thread)

Handle nested position:sticky elements correctly (main thread)

In order to correctly calculate the sticky offset for nested sticky, we
must track the accumulated sticky offset from our ancestor sticky
elements. This accumulation must then be used to adjust the location of
the two constraint rects.

This patch fixed nested position: sticky only for the main thread side.
A compositor fix will follow in a later patch. To ensure that nested
sticky behaves properly in the meantime, we also disallow compositing of
sticky elements if nesting is detected.

BUG=672710
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2708883005
Cr-Commit-Position: refs/heads/master@{#452944}
Committed: https://chromium.googlesource.com/chromium/src/+/b693f94a81551da0334f9fb297813e617d9539c0

[smcgruer, 2017-02-22 15:04:56]

Description was changed from

==========
Handle nested position:sticky elements correctly (main thread)

In order to correctly calculate the sticky offset for nested sticky, we
must track the accumulated sticky offset from our ancestor sticky
elements. This accumulation must then be used to adjust the location of
the two constraint rects.

This patch fixed nested position: sticky only for the main thread side.
A compositor fix will follow in a later patch. To ensure that nested
sticky behaves properly in the meantime, we also disallow compositing of
sticky elements if nesting is detected.

BUG=672710
==========

to

==========
Handle nested position:sticky elements correctly (main thread)

In order to correctly calculate the sticky offset for nested sticky, we
must track the accumulated sticky offset from our ancestor sticky
elements. This accumulation must then be used to adjust the location of
the two constraint rects.

This patch fixed nested position: sticky only for the main thread side.
A compositor fix will follow in a later patch. To ensure that nested
sticky behaves properly in the meantime, we also disallow compositing of
sticky elements if nesting is detected.

BUG=672710
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[smcgruer, 2017-02-22 15:05:14]

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

[commit-bot: I haz the power, 2017-02-22 15:05:32]

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, 2017-02-22 15:08:00]

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

[commit-bot: I haz the power, 2017-02-22 15:08:01]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[smcgruer, 2017-02-22 18:57:55]

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

[commit-bot: I haz the power, 2017-02-22 18:58:37]

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, 2017-02-22 20:48:13]

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

[commit-bot: I haz the power, 2017-02-22 20:48:14]

Dry run: This issue passed the CQ dry run.

[smcgruer, 2017-02-22 21:05:57]

smcgruer@chromium.org changed reviewers:
+ flackr@chromium.org

[smcgruer, 2017-02-22 21:05:58]

There are still a few TODOs to polish off and get right, but I think this is
quite close based on all the discussion on the bigger patch. (Hopefully!)

[flackr, 2017-02-22 22:40:15]

For reference this was split off from
https://codereview.chromium.org/2636253002/

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-deep.html
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-deep.html:4:
internals.settings.setCSSStickyPositionEnabled(true);
nit: use consistent 2-space tab for tests JS and CSS.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-deep.html:66:
finishTestInXFrames(2);
It's much shorter and easier to understand to write out the two rafs I think.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-inline.html
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-inline.html:56:
for (const scroller of document.querySelectorAll('.scroller')) {
There seems to only be one scroller, why the loop?

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-inline.html:69:
<div class="outerSticky">1<div class="innerSticky">2</div></div>
Can you use colored or outlined blocks instead of text?

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:928:
stickyBoxRect.move(scrollOffset);
This seems like a separate bug. Can we land this in a different patch?

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp (right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp:348: // Force
the constraints to be calculated.
Maybe just updateAllLifecyclePhases?

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:348:
// TODO(smcgruer): Copy the required fields for nested sticky in compositor.
You can reference the existing bug, and consider it not fixed until the
compositor side is done too.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp:20:
const LayoutBoxModelObject* o) {
Can we pass through the reference type instead of a pointer?

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp:21:
if (!o || !o->layer()->ancestorOverflowLayer())
o should never be null afaict. Since we're calling this from
CompositingReasonFinder I think that ancestorOverflowLayer should never be null
either should it?

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp:28:
return nullptr;
Can this happen? We only call this function for sticky position objects with
clean compositing inputs right?

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp:260:
!constraints->hasAncestorStickyElement();
stickyConstraintsForLayoutObject can return null but we do not check for null.
Can we remove the null returns from stickyConstraintsForLayoutObject or do we
need to check for null here?

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinderTest.cpp
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinderTest.cpp:302:
EXPECT_NE(NotComposited, outerStickyLayer->compositingState());
Expect PaintsIntoOwnBacking

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h:56:
StickyPositionScrollingConstraints* ancestorStickyBoxConstraints,
const StickyPositionScrollingConstraints*

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h:159:
LayoutBoxModelObject* m_nearestStickyElementShiftingContainingBlock;
bikeshed nit: Can we call these nearestBoxShifting* since they're LayoutBox
pointers rather than elements?

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h:173:
FloatSize m_cachedComputedStickyOffset;
m_localStickyOffset?

But it looks like the code never uses this as far as I can tell, in which case
we should remove it.

[smcgruer, 2017-02-23 20:14:17]

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-deep.html
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-deep.html:4:
internals.settings.setCSSStickyPositionEnabled(true);
On 2017/02/22 22:40:14, flackr wrote:
> nit: use consistent 2-space tab for tests JS and CSS.

Done.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-deep.html:66:
finishTestInXFrames(2);
On 2017/02/22 22:40:14, flackr wrote:
> It's much shorter and easier to understand to write out the two rafs I think.

Done.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-inline.html
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-inline.html:56:
for (const scroller of document.querySelectorAll('.scroller')) {
On 2017/02/22 22:40:14, flackr wrote:
> There seems to only be one scroller, why the loop?

Copy/paste error. Removed.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/sticky/nested/sticky-nested-inline.html:69:
<div class="outerSticky">1<div class="innerSticky">2</div></div>
On 2017/02/22 22:40:14, flackr wrote:
> Can you use colored or outlined blocks instead of text?

Unless you object, I'm actually going to leave this as-is for now based on the
issues with inline elements. Filed crbug.com/695550 to track those.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:928:
stickyBoxRect.move(scrollOffset);
On 2017/02/22 22:40:14, flackr wrote:
> This seems like a separate bug. Can we land this in a different patch?

Certainly, removed this and updated StickyPositionNestedInlineElements in
LayoutBoxModelObjectTest to still work with this bug unfixed. Opened
crbug.com/695576 to track

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp (right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp:348: // Force
the constraints to be calculated.
On 2017/02/22 22:40:14, flackr wrote:
> Maybe just updateAllLifecyclePhases?

It turns out that setBodyInnerHTML(..) does exactly that, so just removed this.

void setBodyInnerHTML(const String& htmlContent) {
  document().body()->setInnerHTML(htmlContent, ASSERT_NO_EXCEPTION);
  document().view()->updateAllLifecyclePhases();
}

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:348:
// TODO(smcgruer): Copy the required fields for nested sticky in compositor.
On 2017/02/22 22:40:15, flackr wrote:
> You can reference the existing bug, and consider it not fixed until the
> compositor side is done too.

Done.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp:20:
const LayoutBoxModelObject* o) {
On 2017/02/22 22:40:15, flackr wrote:
> Can we pass through the reference type instead of a pointer?

Used the original PaintLayer instead, and pulled this out of a helper method
since its not very long anymore...

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp:21:
if (!o || !o->layer()->ancestorOverflowLayer())
On 2017/02/22 22:40:15, flackr wrote:
> o should never be null afaict. Since we're calling this from
> CompositingReasonFinder I think that ancestorOverflowLayer should never be
null
> either should it?

Done.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp:28:
return nullptr;
On 2017/02/22 22:40:15, flackr wrote:
> Can this happen? We only call this function for sticky position objects with
> clean compositing inputs right?

Done.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp:260:
!constraints->hasAncestorStickyElement();
On 2017/02/22 22:40:15, flackr wrote:
> stickyConstraintsForLayoutObject can return null but we do not check for null.
> Can we remove the null returns from stickyConstraintsForLayoutObject or do we
> need to check for null here?

Done.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinderTest.cpp
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinderTest.cpp:302:
EXPECT_NE(NotComposited, outerStickyLayer->compositingState());
On 2017/02/22 22:40:15, flackr wrote:
> Expect PaintsIntoOwnBacking

Done.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h
(right):

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h:56:
StickyPositionScrollingConstraints* ancestorStickyBoxConstraints,
On 2017/02/22 22:40:15, flackr wrote:
> const StickyPositionScrollingConstraints*

Done.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h:159:
LayoutBoxModelObject* m_nearestStickyElementShiftingContainingBlock;
On 2017/02/22 22:40:15, flackr wrote:
> bikeshed nit: Can we call these nearestBoxShifting* since they're LayoutBox
> pointers rather than elements?

Done.

https://codereview.chromium.org/2708883005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h:173:
FloatSize m_cachedComputedStickyOffset;
On 2017/02/22 22:40:15, flackr wrote:
> m_localStickyOffset?
> 
> But it looks like the code never uses this as far as I can tell, in which case
> we should remove it.

Ah, this is from the cc side. Removed for now.

[flackr, 2017-02-24 01:33:33]

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (right):

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:59: if (!o ||
!o->layer()->ancestorOverflowLayer())
Maybe we should pass ancestorOverflowLayer in since it must be the same for all
of the passed in layout objects and it will save us doing this lookup and early
exit check.

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:2083: (mode &
IgnoreStickyOffset)) {
nit: Maybe do the mode check first since most of the time it will be false.

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
(right):

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp:247:
.get(nonConstLayer)
nit: I think it'd be better to do the const conversion inline so as to emphasize
that it is only used for the map lookup.

[smcgruer, 2017-02-24 14:17:43]

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (right):

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:59: if (!o ||
!o->layer()->ancestorOverflowLayer())
On 2017/02/24 01:33:33, flackr wrote:
> Maybe we should pass ancestorOverflowLayer in since it must be the same for
all
> of the passed in layout objects and it will save us doing this lookup and
early
> exit check.

Done, although we still need to null-check |o| as the code is designed to act
sensibly when you have no nearestStickyBoxShifting{StickyBox,ContainingBlock}.

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:2083: (mode &
IgnoreStickyOffset)) {
On 2017/02/24 01:33:33, flackr wrote:
> nit: Maybe do the mode check first since most of the time it will be false.

Done. Also removed the brackets to let it fit on one line; lmk if that's bad
style.

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
(right):

https://codereview.chromium.org/2708883005/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp:247:
.get(nonConstLayer)
On 2017/02/24 01:33:33, flackr wrote:
> nit: I think it'd be better to do the const conversion inline so as to
emphasize
> that it is only used for the map lookup.

Done.

[flackr, 2017-02-24 15:07:37]

LGTM

https://codereview.chromium.org/2708883005/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (right):

https://codereview.chromium.org/2708883005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:1040: //
compositing inputs have been computed?
I don't think we'd want to do this here, as I think this will be called as part
of layout, but it makes sense that we'd want to do this for the web exposed
API's which return position.

https://codereview.chromium.org/2708883005/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2708883005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:2082: if (mode &
IgnoreStickyOffset && isBoxModelObject() && isStickyPositioned()) {
I don't think the object can be sticky position and not a box model object?

[smcgruer, 2017-02-24 15:27:57]

https://codereview.chromium.org/2708883005/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (right):

https://codereview.chromium.org/2708883005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:1040: //
compositing inputs have been computed?
On 2017/02/24 15:07:37, flackr wrote:
> I don't think we'd want to do this here, as I think this will be called as
part
> of layout, but it makes sense that we'd want to do this for the web exposed
> API's which return position.

Would you like me to remove this comment then? It is from the original sticky
CL; I just restored it since I was restoring the original behavior.

https://codereview.chromium.org/2708883005/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2708883005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:2082: if (mode &
IgnoreStickyOffset && isBoxModelObject() && isStickyPositioned()) {
On 2017/02/24 15:07:37, flackr wrote:
> I don't think the object can be sticky position and not a box model object?

Done.

[smcgruer, 2017-02-24 18:31:42]

smcgruer@chromium.org changed reviewers:
+ schenney@google.com

[smcgruer, 2017-02-24 18:31:43]

+schenney for blink OWNERS (Chris is still OOO I think)

[smcgruer, 2017-02-24 18:39:48]

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

[commit-bot: I haz the power, 2017-02-24 18:40:37]

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, 2017-02-24 18:43:36]

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

[commit-bot: I haz the power, 2017-02-24 18:43:37]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Stephen Chennney, 2017-02-24 18:58:11]

On 2017/02/24 18:31:43, smcgruer wrote:
> +schenney for blink OWNERS (Chris is still OOO I think)

Could you rebase and check the trybots again? I'd like coverage of the last
change you made.

[smcgruer, 2017-02-24 19:08:35]

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

[commit-bot: I haz the power, 2017-02-24 19:09:25]

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, 2017-02-24 21:19:51]

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

[commit-bot: I haz the power, 2017-02-24 21:19:52]

Dry run: This issue passed the CQ dry run.

[Stephen Chennney, 2017-02-24 21:42:01]

On 2017/02/24 18:58:11, Stephen Chennney wrote:
> On 2017/02/24 18:31:43, smcgruer wrote:
> > +schenney for blink OWNERS (Chris is still OOO I think)
> 
> Could you rebase and check the trybots again? I'd like coverage of the last
> change you made.

lgtm. Thanks for your patience.

[smcgruer, 2017-02-24 21:57:37]

The CQ bit was checked by smcgruer@chromium.org

[smcgruer, 2017-02-24 21:57:38]

The patchset sent to the CQ was uploaded after l-g-t-m from flackr@chromium.org
Link to the patchset: https://codereview.chromium.org/2708883005/#ps140001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-24 21:58:11]

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, 2017-02-24 22:07:26]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1487973457920790,
"parent_rev": "4e8c7bbcbe48fc7f786b3541ad43362b96d56a35", "commit_rev":
"b693f94a81551da0334f9fb297813e617d9539c0"}

[commit-bot: I haz the power, 2017-02-24 22:08:07]

Description was changed from

==========
Handle nested position:sticky elements correctly (main thread)

In order to correctly calculate the sticky offset for nested sticky, we
must track the accumulated sticky offset from our ancestor sticky
elements. This accumulation must then be used to adjust the location of
the two constraint rects.

This patch fixed nested position: sticky only for the main thread side.
A compositor fix will follow in a later patch. To ensure that nested
sticky behaves properly in the meantime, we also disallow compositing of
sticky elements if nesting is detected.

BUG=672710
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Handle nested position:sticky elements correctly (main thread)

In order to correctly calculate the sticky offset for nested sticky, we
must track the accumulated sticky offset from our ancestor sticky
elements. This accumulation must then be used to adjust the location of
the two constraint rects.

This patch fixed nested position: sticky only for the main thread side.
A compositor fix will follow in a later patch. To ensure that nested
sticky behaves properly in the meantime, we also disallow compositing of
sticky elements if nesting is detected.

BUG=672710
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2708883005
Cr-Commit-Position: refs/heads/master@{#452944}
Committed:
https://chromium.googlesource.com/chromium/src/+/b693f94a81551da0334f9fb29781...
==========

[commit-bot: I haz the power, 2017-02-24 22:08:08]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/b693f94a81551da0334f9fb29781...