New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 634239 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Regression

Blocking:
issue 529938



Sign in to add a comment

Regression: Weird overlapping of cookies is seen in 'cookies set by this page' overlay.

Reported by vku...@etouch.net, Aug 4 2016

Issue description

Chrome Version: 54.0.2817.0 (Official Build) cd4d5eb8d957a3ce7ccf0fa60243eccd8ba47691-refs/heads/master@{#409416}-32/64 bit
OS: Windows (7,8,8.1,10)

What steps will reproduce the problem?
(1)Launch chrome and navigate to http://www.shockwave.com/online/all-games.jsp
(2)Click on "view site information" click on cookies link button and observe the button.


Actual: Weird overlapping of cookies is seen in 'cookies set by this page' overlay.

Expected: Overlapping of cookies should not be seen in 'cookies set by this page' overlay.

This is a regression issue broken in 'M54' and will soon update other info.


 

Comment 1 by vku...@etouch.net, Aug 4 2016

Cc: danakj@chromium.org
Components: Blink>Layout
Labels: hasbisect OS-Linux
Owner: wkorman@chromium.org
Status: Assigned (was: Unconfirmed)
Manual regression range:
Good Build: 54.0.2816.0 
Bad Build: 54.0.2817.0 

Narrow bisect
https://chromium.googlesource.com/chromium/src/+log/df585142df9fd58299ef02b64da3381d8b733cb8..02a9535ecacde9f391e1f6832c5b99dc4e54cc6e?pretty=fuller&n=50

Suspecting: 409291 or 409270 ?
Kindly help to re-assign, if your changes are not cause for this issue.

Note: Issue not seen on Mac OS.
Actual_Result.png
142 KB View Download
Labels: ReleaseBlock-Beta
Adding release block label, please undo if not the case.
Cc: chrishtr@chromium.org vmp...@chromium.org
Components: -Blink>Layout Blink>Paint
This looks due to my change http://crrev.com/1484163002, I can repro synced to my commit and not with the preceding commit. I will look into it shortly.
Cc: wkorman@chromium.org
 Issue 634242  has been merged into this issue.
Suspect some issue with ClipRecorder bounds as it pertains to ui::views::TreeView. Notes:

[19499:19499:0804/133115:ERROR:tree_view.cc(604)] Got canvas clip bounds [clip_rect=-1.000000,-1.000000 386.000000x282.000000].
[19499:19499:0804/133115:ERROR:tree_view.cc(619)] view_content_bounds=0,0 384x280

but dimensions of the tree view scrollable region in actual screenshot are actually ~ 386x124 (excluding scrollbar) or 400x124 (including scrollbar).

Note also that TreeView::OnPaint() overrides View::OnPaint() and does its own thing though this may or may not be related:
https://cs.chromium.org/chromium/src/ui/views/controls/tree/tree_view.cc?sq=package:chromium&dr&rcl=1470324312&l=595

Believe TreeView is used for both bookmarks and the cookie list, and so would explain dupe'd bug as well.
tree-view.png
10.7 KB View Download
Actually, ScrollView contains the TreeView and probably the TreeView is not-incorrectly painting more than we need to allow for scrolling.

The bounds of the scroll view look ok but perhaps there is an issue with nested clips or a prior assumption, now invalid, re: containing layer and its bounds producing an implicit clip.

[20014:20014:0804/141434:ERROR:scroll_view.cc(339)] Scheduling paint [is_bounded=0, viewport bounds=1,1 384x123, localBounds=0,0 401x125].
[20014:20014:0804/141434:ERROR:scroll_view.cc(339)] Scheduling paint [is_bounded=0, viewport bounds=1,1 384x123, localBounds=0,0 401x125].
[20014:20014:0804/141434:ERROR:scroll_view.cc(339)] Scheduling paint [is_bounded=0, viewport bounds=1,1 399x123, localBounds=0,0 401x125].

[20014:20014:0804/141434:ERROR:clip_recorder.cc(54)] Begin clip in layer space=24,127 401x125
[20014:20014:0804/141434:ERROR:clip_recorder.cc(54)] Begin clip in layer space=25,128 384x123
[20014:20014:0804/141434:ERROR:clip_recorder.cc(54)] Begin clip in layer space=25,128 384x349
[20014:20014:0804/141434:ERROR:tree_view.cc(595)] Painting TreeView.
[20014:20014:0804/141434:ERROR:tree_view.cc(604)] Got canvas clip bounds [clip_rect=-1.000000,-1.000000 386.000000x351.000000].
[20014:20014:0804/141434:ERROR:tree_view.cc(619)] view_content_bounds=0,0 384x349
[20014:20014:0804/141434:ERROR:clip_recorder.cc(23)] ClipRecorder destructor [num_closers=1].
[20014:20014:0804/141434:ERROR:clip_recorder.cc(28)] End clip bounds_in_layer=25,128 384x349
[20014:20014:0804/141434:ERROR:clip_recorder.cc(23)] ClipRecorder destructor [num_closers=1].
[20014:20014:0804/141434:ERROR:clip_recorder.cc(28)] End clip bounds_in_layer=25,128 384x123
[20014:20014:0804/141434:ERROR:clip_recorder.cc(54)] Begin clip in layer space=409,128 15x123
[20014:20014:0804/141434:ERROR:clip_recorder.cc(54)] Begin clip in layer space=409,128 15x123
[20014:20014:0804/141434:ERROR:clip_recorder.cc(54)] Begin clip in layer space=409,142 15x33
[20014:20014:0804/141434:ERROR:clip_recorder.cc(23)] ClipRecorder destructor [num_closers=1].
[20014:20014:0804/141434:ERROR:clip_recorder.cc(28)] End clip bounds_in_layer=409,142 15x33
[20014:20014:0804/141434:ERROR:clip_recorder.cc(54)] Begin clip in layer space=409,128 15x14
[20014:20014:0804/141434:ERROR:clip_recorder.cc(23)] ClipRecorder destructor [num_closers=1].
[20014:20014:0804/141434:ERROR:clip_recorder.cc(28)] End clip bounds_in_layer=409,128 15x14
[20014:20014:0804/141434:ERROR:clip_recorder.cc(54)] Begin clip in layer space=409,237 15x14
[20014:20014:0804/141434:ERROR:clip_recorder.cc(23)] ClipRecorder destructor [num_closers=1].
[20014:20014:0804/141434:ERROR:clip_recorder.cc(28)] End clip bounds_in_layer=409,237 15x14
[20014:20014:0804/141434:ERROR:clip_recorder.cc(23)] ClipRecorder destructor [num_closers=1].
[20014:20014:0804/141434:ERROR:clip_recorder.cc(28)] End clip bounds_in_layer=409,128 15x123
[20014:20014:0804/141434:ERROR:clip_recorder.cc(23)] ClipRecorder destructor [num_closers=1].
[20014:20014:0804/141434:ERROR:clip_recorder.cc(28)] End clip bounds_in_layer=409,128 15x123
[20014:20014:0804/141434:ERROR:clip_recorder.cc(23)] ClipRecorder destructor [num_closers=1].
[20014:20014:0804/141434:ERROR:clip_recorder.cc(28)] End clip bounds_in_layer=24,127 401x125

So.. I'm adding a feature which seems to fix this, which is to scroll with Layers in views::ScrollView - https://codereview.chromium.org/2188133002/ . But I wasn't planning on turning it on by default anywhere except Mac just yet. Also, it doesn't address the underlying problem, but if there's no other symptoms, and addressing the underlying problem has bad performance implications, then it may be an option.
@tapted that would be very exciting, to consider further though I see your change is non-trivial and bake time likely warranted.

I've spent the afternoon debugging, but it's a pain as I've not got a reduced test case yet and haven't spent a lot of time in ui/views code to date. Considering either building a fake View > ScrollView > TreeView that demonstrates the issue, or hacking fake data into the cookie list dialog, to make debugging with an empty html doc easier.

I'm not yet sure that there's a fundamental bug outside of ScrollView/TreeView.

Work will continue on the morrow.
I am preparing a revert due to  http://crbug.com/634823  to land today and will continue looking into this to fix for re-land.
Blocking: 529938
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/490867260447e9360b52d5c03417b000490332e0

commit 490867260447e9360b52d5c03417b000490332e0
Author: wkorman <wkorman@chromium.org>
Date: Fri Aug 05 22:24:57 2016

Revert "Raster display item lists via a visual rect RTree."

Meta-revert for http://crrev.com/1484163002 due to bugs found in ui/views and
DevTools.

Original change description:

    Raster display item lists via a visual rect RTree.

    Rather than caching and playing back an entire SkPicture
    when rastering a display item list for a particular
    playback rect, instead retain display items and query
    them via an RTree of their visual rects to find and
    raster only what's needed.

    Display item lists no longer support the notion of a
    bounding "layer rect" with mutable origin.

    DisplayItemListSettings proto is obsolete after this
    change as it's comprised solely of one field to allow
    switching whether to use the aforementioned now-deleted
    cached SkPicture code path. It will be deleted in a
    subsequent patch.

Revert "Raster display item lists via a visual rect RTree."

This reverts commit ccb9e13712b1632b889960d1d85d556c0139fd51.

Revert "Don't clear visual rects when finalizing display item lists for now."

This reverts commit 1adf72a0a0a3e04151cc740d15ab19655b1e7e5e.

Revert "Delete obsolete DisplayItemList::ProcessAppendedItem method definition."

This reverts commit f652746f56c59523b0440cf18b769f8ba779d15d.

BUG= 529938 , 633750 , 633869 , 634239 , 634823 , 634959 
TBR=chrishtr@chromium.org,vmpstr@chromium.org,lushnikov@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2217263003
Cr-Commit-Position: refs/heads/master@{#410190}

[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/blink/web_content_layer_impl.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/blink/web_display_item_list_impl.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/debug/rasterize_and_record_benchmark.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/debug/rasterize_and_record_benchmark.h
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/layers/empty_content_layer_client.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/layers/picture_image_layer.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/layers/picture_layer_impl_unittest.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/playback/discardable_image_map_unittest.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/playback/display_item_list.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/playback/display_item_list.h
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/playback/display_item_list_unittest.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/playback/recording_source.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/proto/display_item.proto
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/test/fake_content_layer_client.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/test/skia_common.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/test/solid_color_content_layer_client.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/trees/layer_tree_host_pixeltest_masks.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/cc/trees/layer_tree_host_pixeltest_tiles.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/ui/compositor/canvas_painter.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/ui/compositor/layer.cc
[modify] https://crrev.com/490867260447e9360b52d5c03417b000490332e0/ui/views/view_unittest.cc

Labels: TE-Verified-M54 TE-Verified-54.0.2824.0
Verified the issue on Latest Dev# 54.0.2824.0 on Windows and Linux and has been fixed and is working as intended.
Hence adding TE-Verified Labels. Also adding screenshot for reference.

Thank You.
634239.jpg
130 KB View Download
Labels: TE-Verified-54.0.2823.0
Just to update, verified the issue on Latest Dev# 54.0.2823.0 for Linux. So adding TE-Verified-54.0.2823.0 for Linux.
Thank You.
The problem is that the clip created by ui::ClipRecorder does not have an
accurate bound, so the compositor rtree is culling it out. This is why scrolled
elements are not getting clipped properly when outside the scroller.

Note that this dialog is implemented in native UI view components, in
ui/views/collected_cookies_views.cc.
FYI I'm currently digging into this.
Update: the bug is that paired display items need to expand to the bounds
of the items they contain.
Proposed fix is an adaptation of http://crrev.com/1814013003 to move visual rect unioning for paired display items into cc:DisplayItemList.
Confirmed that patch in flight http://crrev.com/2230513005 merged with the rtree patch http://crrev.com/2225563002 fixes the broken clipping in shockwave cookie list view.

I'm porting unit tests for the logic move from Blink to cc and will send change for review when complete.
Project Member

Comment 19 by bugdroid1@chromium.org, Aug 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/923cfdd728ac4e0f7f35333c994c79e6f9403625

commit 923cfdd728ac4e0f7f35333c994c79e6f9403625
Author: wkorman <wkorman@chromium.org>
Date: Thu Aug 11 00:04:33 2016

Move visual rect unioning between paired items to cc::DisplayItemList.

http://crrev.com/1814013003 added this for Blink-driven visual rects,
and it turns out we need it for ui/views as well, so we centralize
in cc directly.

Visual rects now only have to be explicitly passed for drawing display
items.

BUG= 634239 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2230513005
Cr-Commit-Position: refs/heads/master@{#411195}

[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/blink/web_display_item_list_impl.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/blink/web_display_item_list_impl.h
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/layers/picture_image_layer.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/playback/display_item_list.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/playback/display_item_list.h
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/playback/display_item_list_unittest.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/playback/display_item_proto_factory.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/test/fake_content_layer_client.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/test/geometry_test_utils.h
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/test/solid_color_content_layer_client.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/trees/layer_tree_host_pixeltest_masks.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/cc/trees/layer_tree_host_pixeltest_tiles.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/chrome/browser/ui/views/autofill/card_unmask_prompt_views.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/ClipDisplayItem.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/ClipPathDisplayItem.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/CompositingDisplayItem.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/DisplayItem.h
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/DisplayItemList.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/DisplayItemList.h
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/DisplayItemListTest.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/FilterDisplayItem.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/FloatClipDisplayItem.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/ScrollDisplayItem.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/Transform3DDisplayItem.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/Source/platform/graphics/paint/TransformDisplayItem.cpp
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/third_party/WebKit/public/platform/WebDisplayItemList.h
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/ui/compositor/clip_recorder.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/ui/compositor/clip_recorder.h
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/ui/compositor/compositing_recorder.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/ui/compositor/compositing_recorder.h
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/ui/compositor/paint_cache.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/ui/compositor/paint_recorder.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/ui/compositor/transform_recorder.cc
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/ui/compositor/transform_recorder.h
[modify] https://crrev.com/923cfdd728ac4e0f7f35333c994c79e6f9403625/ui/views/view.cc

Status: Fixed (was: Assigned)
When we re-land the patch for  crbug.com/529938  shortly, potentially tomorrow, it should now no longer introduce the issue noted in this bug.
Labels: TE-Verified-54.0.2830.0
Tested the issue on Windows 7, Ubuntu 14.04 using 54.0.2830.0.Overlapping of cookies is not seen in 'cookies set by this page' overlay.
Please find attached screenshot.

634239.png
222 KB View Download

Sign in to add a comment