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

Issue 640500 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Crash in cc::draw_property_utils::UpdatePageScaleFactor

Project Member Reported by ClusterFuzz, Aug 24 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5161623809687552

Fuzzer: mbarbella_js_mutation_layout
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: UNKNOWN WRITE
Crash Address: 0x000000000128
Crash State:
  cc::draw_property_utils::UpdatePageScaleFactor
  cc::LayerTreeImpl::PushPageScaleFactorAndLimits
  cc::LayerTreeImpl::PushPageScaleFromMainThread
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_content_shell_drt&range=374754:374870

Minimized Testcase (0.20 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94VQacoOGlbJAKuCqRALSF0ACnpy0_qsR_dqAXAYY4z2WQXncMEHqSwyL2y6CmtDAMBnroxped5deCjBR_ALxB17GNYiANH2yNo5k48AnGgiQnrt9vvO61ZGexdLsM8RO_nvHSLHLrAhStCJtCIpfW_CFw64A?testcase_id=5161623809687552
<script>
        window.internals.setPageScaleFactor(-1);
window.onload = function() {
  if (!window.testRunner)
    return;
  window.testRunner.capturePixelsAsyncThen(function() {
  });
};
  </script>


Issue manually filed by: durga.behera

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Components: Internals>Compositing Tools>Test>FindIt>CorrectResult
Labels: Te-Logged M-53
Owner: khushals...@chromium.org
Status: Assigned (was: Untriaged)
Suspected CLs
================
No CL in the regression range changes the crashed files. The result is the blame information.

Author: weiliangc
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/c97575c770131ce95b1dff0aa3309d8aba795340
Time: Thu Mar 03 18:34:27 2016
The CL last changed line 1552 of file draw_property_utils.cc, which is stack frame 0.

Author: weiliangc
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/c97575c770131ce95b1dff0aa3309d8aba795340
Time: Thu Mar 03 18:34:27 2016
The CL last changed line 749 of file layer_tree_impl.cc, which is stack frame 1.

Author: aelias
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/58eec0810c6412fb9debb83754bd0e5a65cecdc6
Time: Thu Dec 04 01:04:40 2014
The CL last changed line 713 of file layer_tree_impl.cc, which is stack frame 2.

Author: khushalsagar
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/86928f9503d820305efffe338712ab25a3f2f27d
Time: Wed Aug 17 21:49:05 2016
The CL last changed line 390 of file layer_tree.cc, which is stack frame 3.

Author: khushalsagar
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/86928f9503d820305efffe338712ab25a3f2f27d
Time: Wed Aug 17 21:49:05 2016
The CL last changed line 423 of file layer_tree_host.cc, which is stack frame 4.

Author: danakj@chromium.org
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/804c89867062a9583fc80424e10d6603baf6a0a6
Time: Wed Mar 13 16:32:21 2013
The CL last changed line 246 of file single_thread_proxy.cc, which is stack frame 5.

Author: enne
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/98f3a6c5c30f1ecfc934838688ce265455a35ec9
Time: Thu Oct 09 20:09:44 2014
The CL last changed line 545 of file single_thread_proxy.cc, which is stack frame 6.

===================
Suspected Project: chromium
Suspected Component: Internals>Compositing

This is impacting to the latest Stable (52.0.2743.116) & Beta (53.0.2785.70).
From the above CL list suspecting changes made to  file layer_tree_host.cc, which is stack frame 4.
Suspect : https://chromium.googlesource.com/chromium/src/+/86928f9503d820305efffe338712ab25a3f2f27d
khushalsagar@ : Could you please take a look into this if its related to your change.
Cc: khushals...@chromium.org
Owner: weiliangc@chromium.org
This looks more like Wei's territory. Could you help take a look?

If it helps, we are hitting this DCHECK from the crash report,

[1:1:0823/215838:2721510214541:FATAL:draw_property_utils.cc(1524)] Check failed: page_scale_layer->transform_tree_index() >= TransformTree::kRootNodeId (-1 vs. 0)
#0 0x0000004cb381 __interceptor_backtrace
#1 0x7f784be58fcc base::debug::StackTrace::StackTrace()
#2 0x7f784bfcf6e2 logging::LogMessage::~LogMessage()
#3 0x7f7845615112 cc::draw_property_utils::UpdatePageScaleFactorInternal<>()
#4 0x7f7845614b85 cc::draw_property_utils::UpdatePageScaleFactor()
#5 0x7f78457cf624 cc::LayerTreeImpl::PushPageScaleFactorAndLimits()
#6 0x7f78457dadfa cc::LayerTreeImpl::PushPageScaleFromMainThread()
#7 0x7f784564ace4 cc::LayerTree::PushPropertiesTo()
#8 0x7f784568dd58 cc::LayerTreeHost::FinishCommitOnImplThread()
#9 0x7f7845a05283 cc::SingleThreadProxy::DoCommit()
#10 0x7f7845a1271b cc::SingleThreadProxy::CompositeImmediately()
#11 0x7f784569b42e cc::LayerTreeHost::Composite()
#12 0x7f78554bff56 content::RenderWidgetCompositor::SynchronouslyComposite()
Cc: sunxd@chromium.org jaydasika@chromium.org ajuma@chromium.org weiliangc@chromium.org
Owner: sunxd@chromium.org
Looks like the page scale layer's transform node was not set up before we push page scale? Looks like https://chromium.googlesource.com/chromium/src/+/cfccd1b3bb04f45527c34f6f52d3052faab0e1b2
LayerImpl::PushPropertiesTo happens after LayerTree::PushPropertiesTo. So, my guess looking at the stack in comment 2 would be that LayerImpl still hasn't got the new transform tree index at that point (which makes me wonder why we din't see this crash till now!)   

Comment 5 by sunxd@chromium.org, Sep 15 2016

I think this has something to do with the -1 page scale, if we change that line to:
  window.internals.setPageScaleFactor(2);
The crash goes away. I'm still looking at the reason why this is triggered.

Comment 6 by sunxd@chromium.org, Sep 15 2016

The clusterfuzz tries to set the page scale with the following parameters:
page_scale_factor = -1,
min_page_scale_factor = 1,
max_page_scale_factor = 4.

I don't think this is a valid test case. The page scale factor in impl side is stored in SyncedProperty<ScaleGroup<float>>. After pushing, the factor stored is:
{
pending_base = -1,
active_base = -1,
active_delta = -1
}

So Get current value returns 1 instead of -1 which is stored in the property tree. Thus it triggers a transform tree update. (We shall never trigger it when pushing properties) Because the transform index is not pushed to the LayerImpls yet, we ended up with this DCHECK failure.
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 20 2016

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

commit 2b5d011b487c215564e66393bd7dd1e71021ea95
Author: sunxd <sunxd@chromium.org>
Date: Tue Sep 20 19:24:42 2016

Prevent LayoutTest setting -1 page scale factor.

Blink did not clamp page scale factor when it is -1 probably because it
is the initial value. This can result in a clusterfuzz crash as cc tries
to clamp the value it got from blink, and later found that the value (1)
does not match the one (-1) it already stores, cc then tries to update the
value and only to find that the layer tree has not be synced.

This CL prevents clusterfuzz from setting a -1 page scale factor.

BUG= 640500 

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

[modify] https://crrev.com/2b5d011b487c215564e66393bd7dd1e71021ea95/third_party/WebKit/Source/core/testing/Internals.cpp

Project Member

Comment 8 by ClusterFuzz, Sep 21 2016

ClusterFuzz has detected this issue as fixed in range 419755:419848.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5161623809687552

Fuzzer: mbarbella_js_mutation_layout
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: UNKNOWN WRITE
Crash Address: 0x000000000128
Crash State:
  cc::draw_property_utils::UpdatePageScaleFactor
  cc::LayerTreeImpl::PushPageScaleFactorAndLimits
  cc::LayerTreeImpl::PushPageScaleFromMainThread
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_content_shell_drt&range=374754:374870
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_content_shell_drt&range=419755:419848

Minimized Testcase (0.20 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94VQacoOGlbJAKuCqRALSF0ACnpy0_qsR_dqAXAYY4z2WQXncMEHqSwyL2y6CmtDAMBnroxped5deCjBR_ALxB17GNYiANH2yNo5k48AnGgiQnrt9vvO61ZGexdLsM8RO_nvHSLHLrAhStCJtCIpfW_CFw64A?testcase_id=5161623809687552
<script>
        window.internals.setPageScaleFactor(-1);
window.onload = function() {
  if (!window.testRunner)
    return;
  window.testRunner.capturePixelsAsyncThen(function() {
  });
};
  </script>


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 9 by ClusterFuzz, Sep 21 2016

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase is verified as fixed, closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Cc: -khushals...@chromium.org
Project Member

Comment 11 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment