New issue
Advanced search Search tips

Issue 704805 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Main thread scrolling reasons should be correctly recorded after crbug.com/701355

Project Member Reported by yigu@chromium.org, Mar 24 2017

Issue description

Due to  crbug.com/701355 , all style related main thread scrolling reasons stop being recorded. This should be fixed ASAP so that we can keep monitoring the UMA metrics to decide which reasons are our top priorities.
 

Comment 1 by yigu@chromium.org, Mar 24 2017

Components: Blink>Compositing

Comment 2 by yigu@chromium.org, Mar 30 2017

Labels: -Pri-2 Postmortem-Followup Pri-1
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 12 2017

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

commit 76d0929033d6cd3ec36b96d1b4845166e70c9349
Author: yigu <yigu@chromium.org>
Date: Wed Apr 12 17:44:53 2017

Move logic of recording main thread scrolling reasons from cc to
blink::ScrollManager

Due to  crbug.com/701355 , all style related main thread scrolling reasons
stop being recorded. This patch moves the logic of storing the style
related reasons to PLSA and move the recording logic to ScrollManager
because the information on the compositor side is insufficient. The
disabled test due to the previous bug has been enabled again with minor
modification.

BUG= 704805 
TEST=All/NonCompositedMainThreadScrollingReasonTest.*;
EventHandlerTest.NonCompositedMainThreadScrollingReason*

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/cc/input/main_thread_scrolling_reason.h
[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/third_party/WebKit/Source/core/frame/FrameView.h
[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/third_party/WebKit/Source/core/input/EventHandlerTest.cpp
[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/third_party/WebKit/Source/core/input/ScrollManager.cpp
[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/third_party/WebKit/Source/core/input/ScrollManager.h
[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp
[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/third_party/WebKit/Source/web/tests/data/two_scrollable_area.html
[modify] https://crrev.com/76d0929033d6cd3ec36b96d1b4845166e70c9349/ui/events/blink/input_handler_proxy.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 13 2017

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

commit 998f8e571bebd0ace499f8fef81f91daffada15f
Author: jdoerrie <jdoerrie@chromium.org>
Date: Thu Apr 13 08:10:32 2017

Revert of Move logic of recording main thread scrolling reasons from cc to blink::ScrollManager (patchset #17 id:320001 of https://codereview.chromium.org/2773593005/ )

Reason for revert:
Likely cause of webkit_unit_tests failing on chromium.webkit/WebKit Android (Nexus4).

BUG= 711190 

Original issue's description:
> Move logic of recording main thread scrolling reasons from cc to
> blink::ScrollManager
>
> Due to  crbug.com/701355 , all style related main thread scrolling reasons
> stop being recorded. This patch moves the logic of storing the style
> related reasons to PLSA and move the recording logic to ScrollManager
> because the information on the compositor side is insufficient. The
> disabled test due to the previous bug has been enabled again with minor
> modification.
>
> BUG= 704805 
> TEST=All/NonCompositedMainThreadScrollingReasonTest.*;
> EventHandlerTest.NonCompositedMainThreadScrollingReason*
>
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
>
> Review-Url: https://codereview.chromium.org/2773593005
> Cr-Commit-Position: refs/heads/master@{#464073}
> Committed: https://chromium.googlesource.com/chromium/src/+/76d0929033d6cd3ec36b96d1b4845166e70c9349

TBR=flackr@chromium.org,ajuma@chromium.org,bokan@chromium.org,tdresser@chromium.org,jwd@chromium.org,yigu@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 704805 

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

[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/cc/input/main_thread_scrolling_reason.h
[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/third_party/WebKit/Source/core/frame/FrameView.h
[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/third_party/WebKit/Source/core/input/EventHandlerTest.cpp
[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/third_party/WebKit/Source/core/input/ScrollManager.cpp
[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/third_party/WebKit/Source/core/input/ScrollManager.h
[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp
[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/third_party/WebKit/Source/web/tests/data/two_scrollable_area.html
[modify] https://crrev.com/998f8e571bebd0ace499f8fef81f91daffada15f/ui/events/blink/input_handler_proxy.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 13 2017

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

commit b797bf2e48843177bc5657622bb7fd1f09bb9e24
Author: yigu <yigu@chromium.org>
Date: Thu Apr 13 17:50:27 2017

Move logic of recording main thread scrolling reasons from cc to
blink::ScrollManager

Due to  crbug.com/701355 , all style related main thread scrolling reasons
stop being recorded. This patch moves the logic of storing the style
related reasons to PLSA and move the recording logic to ScrollManager
because the information on the compositor side is insufficient. The
disabled test due to the previous bug has been enabled again with minor
modification.

BUG= 704805 
TEST=All/NonCompositedMainThreadScrollingReasonTest.*;
EventHandlerTest.NonCompositedMainThreadScrollingReason*

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2773593005
Cr-Original-Commit-Position: refs/heads/master@{#464073}
Committed: https://chromium.googlesource.com/chromium/src/+/76d0929033d6cd3ec36b96d1b4845166e70c9349
Review-Url: https://codereview.chromium.org/2773593005
Cr-Commit-Position: refs/heads/master@{#464461}

[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/cc/input/main_thread_scrolling_reason.h
[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/third_party/WebKit/Source/core/frame/FrameView.h
[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/third_party/WebKit/Source/core/input/EventHandlerTest.cpp
[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/third_party/WebKit/Source/core/input/ScrollManager.cpp
[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/third_party/WebKit/Source/core/input/ScrollManager.h
[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp
[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/third_party/WebKit/Source/web/tests/data/two_scrollable_area.html
[modify] https://crrev.com/b797bf2e48843177bc5657622bb7fd1f09bb9e24/ui/events/blink/input_handler_proxy.cc

Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Fixed?

Comment 7 by yigu@chromium.org, Apr 28 2017

Status: Fixed (was: Started)
Yes.

Sign in to add a comment