Remove logic about recording style related main thread scroll reasons

Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame scroll on main if there were any
scrollers which needed to scroll on main which it shouldn't. It also caused 34.5% regression in
smoothness.key_mobile_sites_smooth benchmark (crbug.com/693527) .As the issue is blocking
release of M57 stable, it's better to remove the recording logic for now and work
on a correct one in a following patch.

BUG=701355
Review-Url: https://codereview.chromium.org/2766893002
Cr-Commit-Position: refs/heads/master@{#458596}
Committed: https://chromium.googlesource.com/chromium/src/+/d528436cf0ab46778ccd56a439a250e41113fc46

[yigu, 2017-03-21 21:06:25]

Description was changed from

==========
Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread
scrolling reasons
of a scrollable overflow element to the main frame. As the issue is blocking
release of M57 stable, it's better to remove the recording logic for now and
work
on a correct one in a following patch.

BUG=701355
==========

to

==========
Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread
scrolling reasons
of a scrollable overflow element to the main frame. As the issue is blocking
release of M57 stable, it's better to remove the recording logic for now and
work
on a correct one in a following patch.

BUG=701355
==========

[yigu, 2017-03-21 21:06:26]

yigu@chromium.org changed reviewers:
+ flackr@chromium.org, pdr@chromium.org

[yigu, 2017-03-21 21:08:57]

The CQ bit was checked by yigu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 21:09:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2017-03-21 21:11:06]

LGTM

I'm worried about pushing this straight to stable. How long will this have to
bake in the dev channel?

https://codereview.chromium.org/2766893002/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp (right):

https://codereview.chromium.org/2766893002/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp:1107:
TEST_P(StyleRelatedMainThreadScrollingReasonTest, DISABLED_TransparentTest) {
Please add a link to the bug that will fix this here (and above the other
tests).

E.g.,
// TODO(yigu): This test has been disabled due to https://crbug.com/701355.

[flackr, 2017-03-21 21:16:18]

LGTM for a quick easy to merge fix, but I think the description should explain
the problem that this would make the main frame scroll on main if there were any
scrollers which needed to scroll on main and mention the 34.5% regression in
smoothness.key_mobile_sites_smooth benchmark / bug.

[yigu, 2017-03-21 21:25:52]

Description was changed from

==========
Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread
scrolling reasons
of a scrollable overflow element to the main frame. As the issue is blocking
release of M57 stable, it's better to remove the recording logic for now and
work
on a correct one in a following patch.

BUG=701355
==========

to

==========
Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread
scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame
scroll on main if there were any
scrollers which needed to scroll on main which it shouldn't. It also caused
34.5% regression in
smoothness.key_mobile_sites_smooth benchmark (crbug.com/693527) .As the issue is
blocking
release of M57 stable, it's better to remove the recording logic for now and
work
on a correct one in a following patch.

BUG=701355
==========

[yigu, 2017-03-21 21:26:13]

Description was changed from

==========
Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread
scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame
scroll on main if there were any
scrollers which needed to scroll on main which it shouldn't. It also caused
34.5% regression in
smoothness.key_mobile_sites_smooth benchmark (crbug.com/693527) .As the issue is
blocking
release of M57 stable, it's better to remove the recording logic for now and
work
on a correct one in a following patch.

BUG=701355
==========

to

==========
Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread
scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame
scroll on main if there were any

scrollers which needed to scroll on main which it shouldn't. It also caused
34.5% regression in
smoothness.key_mobile_sites_smooth benchmark (crbug.com/693527) .As the issue is
blocking
release of M57 stable, it's better to remove the recording logic for now and
work
on a correct one in a following patch.

BUG=701355
==========

[yigu, 2017-03-21 21:26:55]

Description was changed from

==========
Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread
scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame
scroll on main if there were any

scrollers which needed to scroll on main which it shouldn't. It also caused
34.5% regression in
smoothness.key_mobile_sites_smooth benchmark (crbug.com/693527) .As the issue is
blocking
release of M57 stable, it's better to remove the recording logic for now and
work
on a correct one in a following patch.

BUG=701355
==========

to

==========
Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread
scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame
scroll on main if there were any
scrollers which needed to scroll on main which it shouldn't. It also caused
34.5% regression in
smoothness.key_mobile_sites_smooth benchmark (crbug.com/693527) .As the issue is
blocking
release of M57 stable, it's better to remove the recording logic for now and
work
on a correct one in a following patch.

BUG=701355
==========

[yigu, 2017-03-21 21:27:05]

The CQ bit was checked by yigu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 21:27:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-21 21:54:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-21 21:54:37]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pdr., 2017-03-21 22:07:42]

After some more discussion with flackr, I am convinced our existing test
coverage of this will cover catastrophic failures. There's always a risk of
course, but it's not as high as I thought.

[flackr, 2017-03-21 22:16:30]

The CQ bit was checked by flackr@chromium.org

[flackr, 2017-03-21 22:16:30]

The patchset sent to the CQ was uploaded after l-g-t-m from flackr@chromium.org,
pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2766893002/#ps20001
(title: "Add TODO comment")

[commit-bot: I haz the power, 2017-03-21 22:17:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-21 23:09:30]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490134590009060,
"parent_rev": "f526c497b9eae3805d18f71c1cefdf88cfaf8ab5", "commit_rev":
"d528436cf0ab46778ccd56a439a250e41113fc46"}

[commit-bot: I haz the power, 2017-03-21 23:10:15]

Description was changed from

==========
Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread
scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame
scroll on main if there were any
scrollers which needed to scroll on main which it shouldn't. It also caused
34.5% regression in
smoothness.key_mobile_sites_smooth benchmark (crbug.com/693527) .As the issue is
blocking
release of M57 stable, it's better to remove the recording logic for now and
work
on a correct one in a following patch.

BUG=701355
==========

to

==========
Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread
scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame
scroll on main if there were any
scrollers which needed to scroll on main which it shouldn't. It also caused
34.5% regression in
smoothness.key_mobile_sites_smooth benchmark (crbug.com/693527) .As the issue is
blocking
release of M57 stable, it's better to remove the recording logic for now and
work
on a correct one in a following patch.

BUG=701355

Review-Url: https://codereview.chromium.org/2766893002
Cr-Commit-Position: refs/heads/master@{#458596}
Committed:
https://chromium.googlesource.com/chromium/src/+/d528436cf0ab46778ccd56a439a2...
==========

[commit-bot: I haz the power, 2017-03-21 23:10:16]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/d528436cf0ab46778ccd56a439a2...