Fixed clip resize for document.rootScroller with inertTopControls

Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size as a result of layout. Now that inertTopControls is
becoming the default , we can no longer rely on layout resizing the
rootScroller's LayoutObject - and thus its clipping layer. This isn't a
problem for the default rootScroller since that's the layoutView.  Its
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the *global* rootScroller is marked as
such. Both the PaintLayerCompositor (for iframes and the root frame) and
GraphicsLayerUpdater (for all other scrollers) inspect this bit and use the
root FrameView's size to set the size for its clipping layer.

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

Committed: https://crrev.com/7ab964b30f73e3507ac3a424f56b1a9101b9bca3
Cr-Commit-Position: refs/heads/master@{#434348}

[bokan, 2016-11-12 22:24:28]

Description was changed from

==========
Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size so the issue isn't seen. When inertTopControls is turned
on, the layout height no longer changes when the url bar hides so the
rootScroller's LayoutBox - and thus it's clipping layer - isn't resized. This
isn't a problem for the default rootScroller since that's the layoutView.  It's
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the rootScroller is marked as such and
the scrolling layer's size is special cased on this bit to size to the
LayoutView size.

BUG=505516
==========

to

==========
Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size so the issue isn't seen. When inertTopControls is turned
on, the layout height no longer changes when the url bar hides so the
rootScroller's LayoutBox - and thus it's clipping layer - isn't resized. This
isn't a problem for the default rootScroller since that's the layoutView.  It's
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the rootScroller is marked as such and
the scrolling layer's size is special cased on this bit to size to the
LayoutView size.

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

[bokan, 2016-11-12 22:24:36]

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

[commit-bot: I haz the power, 2016-11-12 22:24:41]

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-11-13 00:45:35]

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

[commit-bot: I haz the power, 2016-11-13 00:45:36]

Dry run: This issue passed the CQ dry run.

[bokan, 2016-11-15 00:11:23]

Description was changed from

==========
Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size so the issue isn't seen. When inertTopControls is turned
on, the layout height no longer changes when the url bar hides so the
rootScroller's LayoutBox - and thus it's clipping layer - isn't resized. This
isn't a problem for the default rootScroller since that's the layoutView.  It's
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the rootScroller is marked as such and
the scrolling layer's size is special cased on this bit to size to the
LayoutView size.

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

to

==========
Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size as a result of layout. Now that inertTopControls is
becoming the default , we can no longer rely on layout resizing the
rootScroller's LayoutObject - and thus it's clipping layer. This isn't a
problem for the default rootScroller since that's the layoutView.  It's
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the rootScroller is marked as such.
Both the PaintLayerCompositor (for iframes and the root frame) and
GraphicsLayerUpdater (for all other scrollers) inspect this bit and use the
root FrameView's size to set the size for the clipping layer.

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

[bokan, 2016-11-15 00:18:13]

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

[commit-bot: I haz the power, 2016-11-15 00:19:12]

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

[bokan, 2016-11-15 00:19:18]

bokan@chromium.org changed reviewers:
+ chrishtr@chromium.org

[bokan, 2016-11-15 00:20:09]

Hi Chris, another root scroller patch, ptal.

https://codereview.chromium.org/2499853002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/page/scrolling/TopDocumentRootScrollerController.cpp
(right):

https://codereview.chromium.org/2499853002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/TopDocumentRootScrollerController.cpp:55:
layer->setNeedsCompositingInputsUpdate();
This isn't really a CompositingInputsUpdate, but I need to mark the
CompositedLayerMapping as needsGraphicsLayerUpdate and I don't think there's a
way to do that from PaintLayer. We can't query compositing state at this point
so I can't ask for the CLM.

Is there a right way to do this, or is this ok? Or should I just disable the
compositing query asserts?

[commit-bot: I haz the power, 2016-11-15 01:54:49]

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

[commit-bot: I haz the power, 2016-11-15 01:54:50]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[bokan, 2016-11-15 12:45:59]

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

[commit-bot: I haz the power, 2016-11-15 12:46:11]

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-11-15 13:19:16]

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

[commit-bot: I haz the power, 2016-11-15 13:19:17]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[bokan, 2016-11-15 17:47:50]

On 2016/11/15 00:20:09, bokan wrote:
> Hi Chris, another root scroller patch, ptal.
> 
>
https://codereview.chromium.org/2499853002/diff/20001/third_party/WebKit/Sour...
> File
>
third_party/WebKit/Source/core/page/scrolling/TopDocumentRootScrollerController.cpp
> (right):
> 
>
https://codereview.chromium.org/2499853002/diff/20001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/core/page/scrolling/TopDocumentRootScrollerController.cpp:55:
> layer->setNeedsCompositingInputsUpdate();
> This isn't really a CompositingInputsUpdate, but I need to mark the
> CompositedLayerMapping as needsGraphicsLayerUpdate and I don't think there's a
> way to do that from PaintLayer. We can't query compositing state at this point
> so I can't ask for the CLM.
> 
> Is there a right way to do this, or is this ok? Or should I just disable the
> compositing query asserts?

Please hold off reviewing while I look at test failures.

[bokan, 2016-11-15 19:41:47]

Patchset #3 (id:40001) has been deleted

[bokan, 2016-11-15 19:48:09]

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

[commit-bot: I haz the power, 2016-11-15 19:48:57]

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-11-15 20:01:58]

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

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

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[bokan, 2016-11-15 21:55:22]

Should be good to review now, remaining failures look unrelated.

[bokan, 2016-11-15 22:02:11]

Description was changed from

==========
Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size as a result of layout. Now that inertTopControls is
becoming the default , we can no longer rely on layout resizing the
rootScroller's LayoutObject - and thus it's clipping layer. This isn't a
problem for the default rootScroller since that's the layoutView.  It's
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the rootScroller is marked as such.
Both the PaintLayerCompositor (for iframes and the root frame) and
GraphicsLayerUpdater (for all other scrollers) inspect this bit and use the
root FrameView's size to set the size for the clipping layer.

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

to

==========
Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size as a result of layout. Now that inertTopControls is
becoming the default , we can no longer rely on layout resizing the
rootScroller's LayoutObject - and thus it's clipping layer. This isn't a
problem for the default rootScroller since that's the layoutView.  It's
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the *global* rootScroller is marked as 
such. Both the PaintLayerCompositor (for iframes and the root frame) and
GraphicsLayerUpdater (for all other scrollers) inspect this bit and use the
root FrameView's size to set the size for its clipping layer.

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

[bokan, 2016-11-16 15:13:35]

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

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

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-11-16 15:54:29]

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

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

Dry run: This issue passed the CQ dry run.

[bokan, 2016-11-16 21:30:58]

ping

[chrishtr, 2016-11-16 23:15:01]

Description was changed from

==========
Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size as a result of layout. Now that inertTopControls is
becoming the default , we can no longer rely on layout resizing the
rootScroller's LayoutObject - and thus it's clipping layer. This isn't a
problem for the default rootScroller since that's the layoutView.  It's
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the *global* rootScroller is marked as 
such. Both the PaintLayerCompositor (for iframes and the root frame) and
GraphicsLayerUpdater (for all other scrollers) inspect this bit and use the
root FrameView's size to set the size for its clipping layer.

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

to

==========
Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size as a result of layout. Now that inertTopControls is
becoming the default , we can no longer rely on layout resizing the
rootScroller's LayoutObject - and thus its clipping layer. This isn't a
problem for the default rootScroller since that's the layoutView.  Its
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the *global* rootScroller is marked as 
such. Both the PaintLayerCompositor (for iframes and the root frame) and
GraphicsLayerUpdater (for all other scrollers) inspect this bit and use the
root FrameView's size to set the size for its clipping layer.

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

[chrishtr, 2016-11-16 23:28:19]

With inert top controls, hiding the controls does not resize the initial
containing block
of the page, but it does resize the viewport. The latter is correct, right? If
it does resize
the viewport, then all elements which are sized or positioned to the viewport
should get
resized / positioned in layout. This change should in turn naturally update all
of the
compositing state without changes to compositing.

In other words, why shouldn't the problem you are fixing be in layout rather
than
compositing? Am I missing something?

[bokan, 2016-11-16 23:39:12]

On 2016/11/16 23:28:19, chrishtr wrote:
> With inert top controls, hiding the controls does not resize the initial
> containing block
> of the page, but it does resize the viewport. The latter is correct, right?
 
Correct.

> If it does resize
> the viewport, then all elements which are sized or positioned to the viewport
> should get
> resized / positioned in layout. This change should in turn naturally update
all
> of the
> compositing state without changes to compositing.
> 
> In other words, why shouldn't the problem you are fixing be in layout rather
> than
> compositing? Am I missing something?

Because this is for elements not sized/positioned by the viewport. i.e. If the
<html> element is set to height: 100%, it takes the height of the ICB. If we now
hide the top controls, the size of the <html>'s box doesn't change but we do
want to see the content in the space revealed by the top controls. If we don't
expand the clip, the bottom of the screen is masked out. Hiding the top controls
in this case doesn't even incur a layout.

[bokan, 2016-11-17 16:36:31]

One more thing I forgot to mention yesterday that's probably important to know:
we restrict the elements that can become a document.rootScroller to those that
are exactly the size of the ICB. You asked yesterday why we should resize a
custom rootScroller element. The answer is because it should behave exactly as
the <body>/<html> element does. If the user sets height=200px on it, it won't
become the rootScroller.

[chrishtr, 2016-11-18 00:37:09]

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

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:1241:
flooredIntSize(layoutBox->document()
Can a LayoutView ever have a non-integral size? I think not. If so, assert that
the flooring has
no effect?

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.cpp (right):

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.cpp:40: if
(!element || !element->layoutObject() || !element->layoutObject()->isBox())
Is it possible for it to be a non-box, or is this just defensive code?

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.cpp:44:
PaintLayer* layer = box->layer();
DCHECK(layer);

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.cpp:54: return
layer->compositor()->rootLayer();
I prefer:

return box->view()->layer()

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.h (right):

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.h:17: PaintLayer*
paintLayerFor(const Element*);
paintLayerForRootScroller(const Element* rootScroller)

and document the behavior about Document vs HTML here briefly also, or refer
to the cpp documentation.

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayer.h (right):

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayer.h:937: void
setIsGlobalRootScroller(bool isGlobalRootScroller) {
Why set these here? Why not just change all call sites to query whether the
PaintLayer in question is the one mentioned by globalRootScrollerController()?
Not caching it here means it can't possibly get out of sync.

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/RootScrollerTest.cpp (right):

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/RootScrollerTest.cpp:929:
mainFrameView()->updateAllLifecyclePhases();
Add a check before this line that the root scroller element does not have
needsLayout.

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/VisualViewportTest.cpp (right):

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/VisualViewportTest.cpp:2377:
webViewImpl->updateAllLifecyclePhases();
Why this change?

[bokan, 2016-11-18 22:30:36]

All comments addressed. ptal.

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

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:1241:
flooredIntSize(layoutBox->document()
On 2016/11/18 00:37:09, chrishtr wrote:
> Can a LayoutView ever have a non-integral size? I think not. If so, assert
that
> the flooring has
> no effect?

I think you're right. I switched it to pixelSnappedIntRect for consistency and
added a DCHECK.

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.cpp (right):

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.cpp:40: if
(!element || !element->layoutObject() || !element->layoutObject()->isBox())
On 2016/11/18 00:37:09, chrishtr wrote:
> Is it possible for it to be a non-box, or is this just defensive code?

If an element is the root scroller it must be a box. I've changed this to be a
DCHECK.

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.cpp:44:
PaintLayer* layer = box->layer();
On 2016/11/18 00:37:09, chrishtr wrote:
> DCHECK(layer);

It's now just used for return.

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.cpp:54: return
layer->compositor()->rootLayer();
On 2016/11/18 00:37:09, chrishtr wrote:
> I prefer:
> 
> return box->view()->layer()

Done.

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.h (right):

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.h:17: PaintLayer*
paintLayerFor(const Element*);
On 2016/11/18 00:37:09, chrishtr wrote:
> paintLayerForRootScroller(const Element* rootScroller)
> 
> and document the behavior about Document vs HTML here briefly also, or refer
> to the cpp documentation.

Done. Ditto for scrollableAreaFor.

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayer.h (right):

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayer.h:937: void
setIsGlobalRootScroller(bool isGlobalRootScroller) {
On 2016/11/18 00:37:09, chrishtr wrote:
> Why set these here? Why not just change all call sites to query whether the
> PaintLayer in question is the one mentioned by globalRootScrollerController()?
> Not caching it here means it can't possibly get out of sync.

Yah, that's probably better. Fixed.

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/RootScrollerTest.cpp (right):

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/RootScrollerTest.cpp:929:
mainFrameView()->updateAllLifecyclePhases();
On 2016/11/18 00:37:09, chrishtr wrote:
> Add a check before this line that the root scroller element does not have
> needsLayout.

Done.

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/VisualViewportTest.cpp (right):

https://codereview.chromium.org/2499853002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/VisualViewportTest.cpp:2377:
webViewImpl->updateAllLifecyclePhases();
On 2016/11/18 00:37:09, chrishtr wrote:
> Why this change?

The viewportSizeChange call now causes a paint invalidation because of the
expanded clip. Turning on setTracksPaintInvalidations below asserts that
everything's clean.

[chrishtr, 2016-11-19 01:10:26]

lgtm

https://codereview.chromium.org/2499853002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/FrameView.h (right):

https://codereview.chromium.org/2499853002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/FrameView.h:417: PaintLayer* layer() const
override;
Move this down next to the other methods for the ScrollableArea interface.

[bokan, 2016-11-19 20:03:46]

bokan@chromium.org changed reviewers:
+ jbroman@chromium.org

[bokan, 2016-11-19 20:03:47]

+jbroman@ for Source/platform

https://codereview.chromium.org/2499853002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/FrameView.h (right):

https://codereview.chromium.org/2499853002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/FrameView.h:417: PaintLayer* layer() const
override;
On 2016/11/19 01:10:26, chrishtr wrote:
> Move this down next to the other methods for the ScrollableArea interface.

Done.

[jbroman, 2016-11-19 20:20:59]

chrishtr's in the root blink owners, so his approval powers should be sufficient
here.

platform rs lgtm nonetheless

[bokan, 2016-11-19 20:22:33]

On 2016/11/19 20:20:59, jbroman wrote:
> chrishtr's in the root blink owners, so his approval powers should be
sufficient
> here.

Ah, right, the Chromite Butler extension doesn't check the root file...thanks.

[bokan, 2016-11-19 20:22:39]

The CQ bit was checked by bokan@chromium.org

[bokan, 2016-11-19 20:22:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/2499853002/#ps120001
(title: "Address comment")

[commit-bot: I haz the power, 2016-11-19 20:22:52]

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

[bokan, 2016-11-19 20:24:16]

The CQ bit was unchecked by bokan@chromium.org

[bokan, 2016-11-24 00:05:41]

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

[commit-bot: I haz the power, 2016-11-24 00:06:36]

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-11-24 02:08:29]

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

[commit-bot: I haz the power, 2016-11-24 02:08:30]

Dry run: Try jobs failed on following builders:
  blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[bokan, 2016-11-24 02:14:32]

The CQ bit was checked by bokan@chromium.org

[bokan, 2016-11-24 02:14:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org, jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2499853002/#ps140001
(title: "Fixed typo")

[commit-bot: I haz the power, 2016-11-24 02:15:00]

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

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

This CL has an open dependency (Issue 2519163003 Patch 60001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[bokan, 2016-11-24 12:06:39]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2016-11-24 12:06:57]

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-24 14:08:47]

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

[commit-bot: I haz the power, 2016-11-24 14:08:49]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[bokan, 2016-11-24 16:32:52]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2016-11-24 16:33:16]

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-24 16:39:23]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1480005172886770,
"parent_rev": "655fbca9fecc3c5a60eeb56092944d3e2d79ae9e", "commit_rev":
"533474f1b84b6b720146eaad9d4c8a728d4de21c"}

[commit-bot: I haz the power, 2016-11-24 16:39:46]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2016-11-24 16:43:47]

Description was changed from

==========
Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size as a result of layout. Now that inertTopControls is
becoming the default , we can no longer rely on layout resizing the
rootScroller's LayoutObject - and thus its clipping layer. This isn't a
problem for the default rootScroller since that's the layoutView.  Its
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the *global* rootScroller is marked as 
such. Both the PaintLayerCompositor (for iframes and the root frame) and
GraphicsLayerUpdater (for all other scrollers) inspect this bit and use the
root FrameView's size to set the size for its clipping layer.

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

to

==========
Fixed clip resize for document.rootScroller with inertTopControls

This fixes the issue where -- with a non default document.rootScroller and
inertTopControls on -- hiding the top controls would not expand the clip
vertically. This resulted in a white rect at the bottom of the screen.

With inertTopControls off, the document.rootScroller's LayoutBox always resizes
to the viewport size as a result of layout. Now that inertTopControls is
becoming the default , we can no longer rely on layout resizing the
rootScroller's LayoutObject - and thus its clipping layer. This isn't a
problem for the default rootScroller since that's the layoutView.  Its
clipping rect is the FrameView which is resized as a result of top control
movement.

This patch extends the URL bar clip resize behavior to arbitrary rootScroller
elements. The PaintLayer that contains the *global* rootScroller is marked as
such. Both the PaintLayerCompositor (for iframes and the root frame) and
GraphicsLayerUpdater (for all other scrollers) inspect this bit and use the
root FrameView's size to set the size for its clipping layer.

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

Committed: https://crrev.com/7ab964b30f73e3507ac3a424f56b1a9101b9bca3
Cr-Commit-Position: refs/heads/master@{#434348}
==========

[commit-bot: I haz the power, 2016-11-24 16:43:48]

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