Fix flattening for scrollable areas.

Fix flattening for scrollable areas.

Previously we flattened the scrollingContentsLayer, but not the
graphics or scrolling layers. This resulted in incorrect hit testing
sometimes.

BUG=543655
Committed: https://crrev.com/9653a8175a97e6f8fa54219c6294ddb71033bfdf
Cr-Commit-Position: refs/heads/master@{#364120}

[tdresser, 2015-10-30 20:01:15]

tdresser@chromium.org changed reviewers:
+ vollick@chromium.org

[tdresser, 2015-10-30 20:01:15]

I'm not completely happy with the testing here, but I think it's probably good
enough.

Ideally we'd actually assert in the browsertest that we're performing compositor
scrolling, instead of just verifying that the scroll behaves correctly.

Any thoughts?

[tdresser, 2015-11-02 15:44:31]

On 2015/10/30 20:01:15, tdresser wrote:
> I'm not completely happy with the testing here, but I think it's probably good
> enough.
> 
> Ideally we'd actually assert in the browsertest that we're performing
compositor
> scrolling, instead of just verifying that the scroll behaves correctly.
> 
> Any thoughts?

I think all the test failures are people asserting on what should flatten, but
I'll double check.

[tdresser, 2015-11-06 20:17:45]

On 2015/11/02 15:44:31, tdresser wrote:
> On 2015/10/30 20:01:15, tdresser wrote:
> > I'm not completely happy with the testing here, but I think it's probably
good
> > enough.
> > 
> > Ideally we'd actually assert in the browsertest that we're performing
> compositor
> > scrolling, instead of just verifying that the scroll behaves correctly.
> > 
> > Any thoughts?
> 
> I think all the test failures are people asserting on what should flatten, but
> I'll double check.

Yeah, test failures look like they're just due to changing what should flatten.
vollick@, does this look reasonable?

Is there any easy way to get the trybots to give you a text diff of all the
layout tests that failed?

[Ian Vollick, 2015-11-06 20:31:58]

On 2015/11/06 20:17:45, tdresser wrote:
> On 2015/11/02 15:44:31, tdresser wrote:
> > On 2015/10/30 20:01:15, tdresser wrote:
> > > I'm not completely happy with the testing here, but I think it's probably
> good
> > > enough.
> > > 
> > > Ideally we'd actually assert in the browsertest that we're performing
> > compositor
> > > scrolling, instead of just verifying that the scroll behaves correctly.
> > > 
> > > Any thoughts?
> > 
> > I think all the test failures are people asserting on what should flatten,
but
> > I'll double check.
> 
> Yeah, test failures look like they're just due to changing what should
flatten.
> vollick@, does this look reasonable?
> 
> Is there any easy way to get the trybots to give you a text diff of all the
> layout tests that failed?

Yep. Check out the archive_webkit_tests_results step and click the
layout_test_results link.

For this build,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...

It'll take you here
https://storage.googleapis.com/chromium-layout-test-archives/win_chromium_rel...

[tdresser, 2015-11-10 17:27:19]

On 2015/11/06 20:31:58, vollick wrote:
> On 2015/11/06 20:17:45, tdresser wrote:
> > On 2015/11/02 15:44:31, tdresser wrote:
> > > On 2015/10/30 20:01:15, tdresser wrote:
> > > > I'm not completely happy with the testing here, but I think it's
probably
> > good
> > > > enough.
> > > > 
> > > > Ideally we'd actually assert in the browsertest that we're performing
> > > compositor
> > > > scrolling, instead of just verifying that the scroll behaves correctly.
> > > > 
> > > > Any thoughts?
> > > 
> > > I think all the test failures are people asserting on what should flatten,
> but
> > > I'll double check.
> > 
> > Yeah, test failures look like they're just due to changing what should
> flatten.
> > vollick@, does this look reasonable?
> > 
> > Is there any easy way to get the trybots to give you a text diff of all the
> > layout tests that failed?
> 
> Yep. Check out the archive_webkit_tests_results step and click the
> layout_test_results link.
> 
> For this build,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
> 
> It'll take you here
>
https://storage.googleapis.com/chromium-layout-test-archives/win_chromium_rel...

This looks correct to me.

I haven't landed a change that requires rebaselining before.

If I understand correctly, I need to set NeedsRebaseline in TestExpectations for
every file that needs rebaselining.
Does it make sense for you to review before or after I've updated
TestExpectations?

[tdresser, 2015-11-10 17:27:21]

On 2015/11/06 20:31:58, vollick wrote:
> On 2015/11/06 20:17:45, tdresser wrote:
> > On 2015/11/02 15:44:31, tdresser wrote:
> > > On 2015/10/30 20:01:15, tdresser wrote:
> > > > I'm not completely happy with the testing here, but I think it's
probably
> > good
> > > > enough.
> > > > 
> > > > Ideally we'd actually assert in the browsertest that we're performing
> > > compositor
> > > > scrolling, instead of just verifying that the scroll behaves correctly.
> > > > 
> > > > Any thoughts?
> > > 
> > > I think all the test failures are people asserting on what should flatten,
> but
> > > I'll double check.
> > 
> > Yeah, test failures look like they're just due to changing what should
> flatten.
> > vollick@, does this look reasonable?
> > 
> > Is there any easy way to get the trybots to give you a text diff of all the
> > layout tests that failed?
> 
> Yep. Check out the archive_webkit_tests_results step and click the
> layout_test_results link.
> 
> For this build,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
> 
> It'll take you here
>
https://storage.googleapis.com/chromium-layout-test-archives/win_chromium_rel...

This looks correct to me.

I haven't landed a change that requires rebaselining before.

If I understand correctly, I need to set NeedsRebaseline in TestExpectations for
every file that needs rebaselining.
Does it make sense for you to review before or after I've updated
TestExpectations?

[Ian Vollick, 2015-11-11 22:05:16]

On 2015/11/10 17:27:21, tdresser wrote:
> On 2015/11/06 20:31:58, vollick wrote:
> > On 2015/11/06 20:17:45, tdresser wrote:
> > > On 2015/11/02 15:44:31, tdresser wrote:
> > > > On 2015/10/30 20:01:15, tdresser wrote:
> > > > > I'm not completely happy with the testing here, but I think it's
> probably
> > > good
> > > > > enough.
> > > > > 
> > > > > Ideally we'd actually assert in the browsertest that we're performing
> > > > compositor
> > > > > scrolling, instead of just verifying that the scroll behaves
correctly.
> > > > > 
> > > > > Any thoughts?
> > > > 
> > > > I think all the test failures are people asserting on what should
flatten,
> > but
> > > > I'll double check.
> > > 
> > > Yeah, test failures look like they're just due to changing what should
> > flatten.
> > > vollick@, does this look reasonable?
> > > 
> > > Is there any easy way to get the trybots to give you a text diff of all
the
> > > layout tests that failed?
> > 
> > Yep. Check out the archive_webkit_tests_results step and click the
> > layout_test_results link.
> > 
> > For this build,
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
> > 
> > It'll take you here
> >
>
https://storage.googleapis.com/chromium-layout-test-archives/win_chromium_rel...
> 
> This looks correct to me.
> 
> I haven't landed a change that requires rebaselining before.
> 
> If I understand correctly, I need to set NeedsRebaseline in TestExpectations
for
> every file that needs rebaselining.
> Does it make sense for you to review before or after I've updated
> TestExpectations?

You should add a patchset with the new test expectations for one platform and do
what you say (adding NeedsRebaseline) for the others.

[tdresser, 2015-12-03 15:13:47]

Sorry for the massive delay here.
PTAL

There are a few bounds changes that seem a bit fishy to me, which I'm not really
sure how to interpret.

See selection-gaps-toggling-with-scrolling-contents-expected.txt
(https://codereview.chromium.org/1407383005/diff/100001/third_party/WebKit/Lay...)
for an example.

How does this look?

[tdresser, 2015-12-03 19:24:37]

I've verified that this doesn't cause any pixel diffs on any of the 6 tests with
bounds that changed.

platform/linux/compositing/overflow/selection-gaps-after-removing-scrolling-contents-expected.txt
platform/linux/compositing/overflow/selection-gaps-toggling-expected.txt
platform/linux/compositing/overflow/selection-gaps-toggling-with-scrolling-contents-expected.txt
platform/linux/virtual/prefer_compositing_to_lcd_text/compositing/overflow/selection-gaps-after-removing-scrolling-contents-expected.txt
platform/linux/virtual/prefer_compositing_to_lcd_text/compositing/overflow/selection-gaps-toggling-expected.txt
platform/linux/virtual/prefer_compositing_to_lcd_text/compositing/overflow/selection-gaps-toggling-with-scrolling-contents-expected.txt

[Ian Vollick, 2015-12-04 03:39:49]

On 2015/12/03 19:24:37, tdresser wrote:
> I've verified that this doesn't cause any pixel diffs on any of the 6 tests
with
> bounds that changed.
> 
>
platform/linux/compositing/overflow/selection-gaps-after-removing-scrolling-contents-expected.txt
> platform/linux/compositing/overflow/selection-gaps-toggling-expected.txt
>
platform/linux/compositing/overflow/selection-gaps-toggling-with-scrolling-contents-expected.txt
>
platform/linux/virtual/prefer_compositing_to_lcd_text/compositing/overflow/selection-gaps-after-removing-scrolling-contents-expected.txt
>
platform/linux/virtual/prefer_compositing_to_lcd_text/compositing/overflow/selection-gaps-toggling-expected.txt
>
platform/linux/virtual/prefer_compositing_to_lcd_text/compositing/overflow/selection-gaps-toggling-with-scrolling-contents-expected.txt

Looks like the mac test failures are real. Just more TestExpectations?

[tdresser, 2015-12-04 18:01:42]

Yup, fixed.

[Ian Vollick, 2015-12-05 00:57:38]

On 2015/12/04 18:01:42, tdresser wrote:
> Yup, fixed.

Great, thanks! lgtm.

[tdresser, 2015-12-07 14:33:36]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-12-07 14:33:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1407383005/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1407383005/140001

[commit-bot: I haz the power, 2015-12-07 16:06:26]

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

[commit-bot: I haz the power, 2015-12-07 16:06:26]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[tdresser, 2015-12-07 20:29:43]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-12-07 20:31:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1407383005/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1407383005/140001

[commit-bot: I haz the power, 2015-12-07 21:08:50]

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

[commit-bot: I haz the power, 2015-12-07 21:08:51]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[tdresser, 2015-12-08 15:16:32]

The CQ bit was checked by tdresser@chromium.org

[tdresser, 2015-12-08 15:16:36]

The CQ bit was unchecked by tdresser@chromium.org

[tdresser, 2015-12-08 15:19:47]

Patchset #9 (id:160001) has been deleted

[tdresser, 2015-12-08 15:20:00]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-12-08 15:20:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1407383005/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1407383005/140001

[commit-bot: I haz the power, 2015-12-08 15:38:00]

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

[commit-bot: I haz the power, 2015-12-08 15:38:01]

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

[tdresser, 2015-12-08 15:40:05]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-12-08 15:41:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1407383005/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1407383005/140001

[commit-bot: I haz the power, 2015-12-08 16:00:18]

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

[commit-bot: I haz the power, 2015-12-08 16:00:19]

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

[tdresser, 2015-12-08 16:19:51]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-12-08 16:20:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1407383005/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1407383005/140001

[commit-bot: I haz the power, 2015-12-08 17:00:50]

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

[commit-bot: I haz the power, 2015-12-08 17:00:53]

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

[tdresser, 2015-12-08 17:17:23]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-12-08 17:18:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1407383005/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1407383005/140001

[commit-bot: I haz the power, 2015-12-08 17:35:22]

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

[commit-bot: I haz the power, 2015-12-08 17:35:23]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tdresser, 2015-12-08 18:06:33]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-12-08 18:08:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1407383005/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1407383005/140001

[commit-bot: I haz the power, 2015-12-08 18:25:34]

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

[commit-bot: I haz the power, 2015-12-08 18:25:35]

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

[tdresser, 2015-12-09 15:09:07]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-12-09 15:09:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1407383005/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1407383005/140001

[commit-bot: I haz the power, 2015-12-09 15:14:39]

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

[commit-bot: I haz the power, 2015-12-09 15:14:40]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tdresser, 2015-12-09 17:32:38]

The CQ bit was checked by tdresser@chromium.org

[tdresser, 2015-12-09 17:32:38]

The patchset sent to the CQ was uploaded after l-g-t-m from vollick@chromium.org
Link to the patchset: https://codereview.chromium.org/1407383005/#ps180001
(title: "Rebase.")

[commit-bot: I haz the power, 2015-12-09 17:37:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1407383005/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1407383005/180001

[commit-bot: I haz the power, 2015-12-09 19:18:50]

Committed patchset #9 (id:180001)

[commit-bot: I haz the power, 2015-12-09 19:19:51]

Description was changed from

==========
Fix flattening for scrollable areas.

Previously we flattened the scrollingContentsLayer, but not the
graphics or scrolling layers. This resulted in incorrect hit testing
sometimes.

BUG=543655
==========

to

==========
Fix flattening for scrollable areas.

Previously we flattened the scrollingContentsLayer, but not the
graphics or scrolling layers. This resulted in incorrect hit testing
sometimes.

BUG=543655

Committed: https://crrev.com/9653a8175a97e6f8fa54219c6294ddb71033bfdf
Cr-Commit-Position: refs/heads/master@{#364120}
==========

[commit-bot: I haz the power, 2015-12-09 19:19:52]

Patchset 9 (id:??) landed as
https://crrev.com/9653a8175a97e6f8fa54219c6294ddb71033bfdf
Cr-Commit-Position: refs/heads/master@{#364120}