New issue
Advanced search Search tips

Issue 826455 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task

Blocking:
issue 842238



Sign in to add a comment

Remove cc::Layer::client_rawptr and always do safe LayerClient access.

Project Member Reported by flackr@chromium.org, Mar 27 2018

Issue description

In https://chromium-review.googlesource.com/c/chromium/src/+/982249/ I added client_rawptr as we use the client pointer during commit, at a time when it is technically safe to do so but base::WeakPtr doesn't know it's safe to do so.

We shouldn't need to access the client via an unchecked pointer. We should be able to fix this in one of two ways:
1) Create a special pointer type that is aware of the main thread blocking on the compositor which allows the reference check, or
2) Take the debug info during the commit on the main thread side for layers which will push properties on the compositor side.
 

Comment 1 by flackr@chromium.org, Apr 25 2018

Cc: flackr@chromium.org
Owner: ----
Status: Available (was: Assigned)
Blocking: 842238
Components: Blink>Paint>Invalidation
Labels: -Pri-3 Pri-2
Owner: wangxianzhu@chromium.org
Status: Assigned (was: Available)
Using client_rawptr to call back into blink breaks blink's single-threaded principle and may cause unexpected results when the main thread is accessing the same data, unless we add lock in blink which is not desirable.

I encountered this today during work for  bug 842238 , and will fix it along with it.
Project Member

Comment 3 by bugdroid1@chromium.org, May 18 2018

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

commit 71b6abec310f1025bd2147b6b0bc2bb7904a1c29
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Fri May 18 18:24:21 2018

[PE] Main thread LayerClient::TakeDebugInfo()

One of LayerClient implementations is blink::GraphicsLayer which doesn't
multi-thread, but previously LayerClient::TakeDebugInfo() was called
from the compositor thread.

Now let LayerTreeHost::WillCommit() call Layer::UpdateDebugInfo() which
calls LayerClient::TakeDebugInfo() and saves the result into inputs_,
in the main thread. Then PushPropertiesTo() will send the saved debug
info to the impl side.

Bug:  826455 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I63d3a9285c534f16567c27a51a6e5f17f6bb415e
Reviewed-on: https://chromium-review.googlesource.com/1066210
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559964}
[modify] https://crrev.com/71b6abec310f1025bd2147b6b0bc2bb7904a1c29/cc/layers/layer.cc
[modify] https://crrev.com/71b6abec310f1025bd2147b6b0bc2bb7904a1c29/cc/layers/layer.h
[modify] https://crrev.com/71b6abec310f1025bd2147b6b0bc2bb7904a1c29/cc/trees/layer_tree_host.cc

Status: Fixed (was: Assigned)

Sign in to add a comment