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

Issue 633750 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Add DisplayItemListSettings::use_remote_compositing to allow optional clearing of visual rects.

Project Member Reported by wkorman@chromium.org, Aug 2 2016

Issue description

Contacted via email:

I have being having an issue getting a clean Blimp build to run on Android, and I kept getting this failure: 

Check failed: inputs_.items.size() == inputs_.visual_rects.size(). items.size() 1 visual_rects.size() 0

It seems like your recent change (CL 1484163002) may be the cause, as this does not occur on in the CL directly before it. Could you please investigate or revert this as soon as possible, as it is blocking me on multiple issues. 

Thanks,
CJ

 Here's the stacktrace: 

[1:1:0802/152735:FATAL:display_item_list.cc(104)] Check failed: inputs_.items.size() == inputs_.visual_rects.size(). items.size() 1 visual_rects.size() 0
#0 0x7f30a7487f2e base::debug::StackTrace::StackTrace()
#1 0x7f30a74ef57f logging::LogMessage::~LogMessage()
#2 0x7f30a0d870ac cc::DisplayItemList::ToProtobuf()
#3 0x7f30a0d91ecd cc::RecordingSource::ToProtobuf()
#4 0x7f30a0cbd8aa cc::PictureLayer::LayerSpecificPropertiesToProto()
#5 0x7f30a0c88e94 cc::Layer::ToLayerPropertiesProto()
#6 0x7f30a0cacd6f cc::LayerProtoConverter::SerializeLayerProperties()
#7 0x7f30a0ee1e94 cc::LayerTreeHost::ToProtobufForCommit()
#8 0x7f30a0fb458f cc::RemoteChannelMain::NotifyReadyToCommitOnImpl()
#9 0x7f30a0f9daff cc::ProxyMain::BeginMainFrame()
#10 0x7f30a0fb2f13 cc::RemoteChannelMain::HandleProto()
#11 0x7f30a0fb29ab cc::RemoteChannelMain::OnProtoReceived()
#12 0x7f30a5824520 content::RenderWidgetCompositor::OnHandleCompositorProto()
#13 0x7f30a59cf79d content::RenderWidget::OnHandleCompositorProto()
 
Cc: nyquist@chromium.org
I'm looking for instructions on how to reproduce, I've not yet build or tried Blimp myself. Am reading http://go/blimp currently.

You could try commenting out the DCHECK that's failing in display_item_list.cc:104 but I expect you'll find that rendering doesn't do the right thing since without visual rects to go with the display items rastering will probably go awry.

...happens on startup, as it navigates to the homepage.

% ninja -C out/Engine -j 2000 blimp_engine_app && ./out/Engine/blimp_engine_app --user-data-dir=/tmp/blimpengine --use-remote-compositing  --blimp-client-token-path=blimp/test/data/test_client_token --disable-cached-picture-raster

The error occurs when I am connecting a Client running on the phone to this engine. In order to run the client, please run the following from the src directory: 

% adb push blimp/test/data/test_client_token /data/data/org.chromium.blimp/blimp_client_token
% adb logcat -c && ninja -C out/Client blimp blimp_tests -j2000 && build/android/adb_install_apk.py out/Client/apks/Blimp.apk && build/android/adb_run_blimp_client && adb logcat

More info at https://cs.chromium.org/chromium/src/blimp/docs/running.md

Comment 3 Deleted

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 3 2016

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

commit 1adf72a0a0a3e04151cc740d15ab19655b1e7e5e
Author: wkorman <wkorman@chromium.org>
Date: Wed Aug 03 01:46:27 2016

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

Breaks Blimp, will explore putting it back behind a switch separately.

BUG= 633750 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
TBR=vmpstr@chromium.org

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

[modify] https://crrev.com/1adf72a0a0a3e04151cc740d15ab19655b1e7e5e/cc/playback/display_item_list.cc

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 7 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

Blocking: -529938
Cc: khushals...@chromium.org
Labels: -Pri-1 Pri-2
Summary: Add DisplayItemListSettings::use_remote_compositing to allow optional clearing of visual rects. (was: DCHECK visual rect fails in display item list for Blimp.)
Currently we've commented out code that is intended to clear the vector of visual rects as part of DisplayItemList::Finalize() to save memory. We'd like to restore that behavior for all cases except Blimp (and I think for Blimp it's only the engine that's affected). Per a comment by khushalsagar@ in http://crrev.com/2204053003: "...use the flag above [switches::kUseRemoteCompositing] to have a setting to side-step the entire Finalize step altogether."

Proposal:

- add field to DisplayItemListSettings populated via above flag (will likely name use_remote_compositing for consistency, but if there's a better thought let me know)

- if field is true, we don't actually need an rtree as Blimp will not be using the display item lists beyond serializing them to a proto. We need the visual rects to hang around for proto serialization, but we can skip rtree build.

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 2 2017

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

commit e653fd0dd153d16d947baf92509c8a586eaec50c
Author: wkorman <wkorman@chromium.org>
Date: Thu Mar 02 00:08:56 2017

Restore swap to clear display item list visual rects.

BUG= 633750 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/e653fd0dd153d16d947baf92509c8a586eaec50c/cc/playback/display_item_list.cc

Status: Fixed (was: Started)

Sign in to add a comment