Refactor PLSA scrollbar existence calculation.

Refactor PLSA scrollbar existence calculation.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/bf81e57ced894452410821516aa5eb94f0eb3d97
Cr-Commit-Position: refs/heads/master@{#426807}

[Eric Seckler, 2016-10-14 17:05:17]

Description was changed from

==========
Refactor PLSA scrollbar existence calculation.

BUG=
==========

to

==========
Refactor PLSA scrollbar existence calculation.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Eric Seckler, 2016-10-14 17:31:28]

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

[Eric Seckler, 2016-10-14 17:31:35]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-10-14 17:32:21]

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, 2016-10-14 17:53:51]

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

[commit-bot: I haz the power, 2016-10-14 17:53:52]

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_...)
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[Eric Seckler, 2016-10-17 08:17:49]

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

[commit-bot: I haz the power, 2016-10-17 08:18:15]

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, 2016-10-17 09:44:27]

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

[commit-bot: I haz the power, 2016-10-17 09:44:28]

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

[Eric Seckler, 2016-10-17 17:38:44]

Patchset #2 (id:40001) has been deleted

[Eric Seckler, 2016-10-17 17:38:51]

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

[commit-bot: I haz the power, 2016-10-17 17:39:17]

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, 2016-10-17 19:16:37]

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

[commit-bot: I haz the power, 2016-10-17 19:16:37]

Dry run: Exceeded global retry quota

[Eric Seckler, 2016-10-17 19:47:44]

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

[commit-bot: I haz the power, 2016-10-17 19:48:33]

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, 2016-10-17 21:25:55]

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

[commit-bot: I haz the power, 2016-10-17 21:25:56]

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

[Eric Seckler, 2016-10-18 08:56:20]

eseckler@chromium.org changed reviewers:
+ skobes@chromium.org

[Eric Seckler, 2016-10-18 08:56:20]

Hey Steve,

Here's an attempt at extracting the scrollbar existence calculation from
updateAfterLayout/StyleChange in PLSA. There are still three invalidation
layouttests failing - I think those are OK though (final scrollbar existence is
the same as before this patch) and need updated expectations. WDYT?

Failing tests:
https://storage.googleapis.com/chromium-layout-test-archives/linux_chromium_r...

Btw, this doesn't solve any of the issues mentioned in crbug.com/625300 and
alike. If you think that this logic would change with layoutNG and this
refactoring wouldn't be useful for this, let me know ... :)

Cheers,
Eric

[skobes, 2016-10-20 17:30:24]

This looks great, thanks for doing it.

It would be good to understand why the invalidation tests fail.  Are we adding
extra invalidations somehow?

[Eric Seckler, 2016-10-20 17:37:22]

On 2016/10/20 17:30:24, skobes wrote:
> This looks great, thanks for doing it.
> 
> It would be good to understand why the invalidation tests fail.  Are we adding
> extra invalidations somehow?

To me, it looks like we are actually seeing fewer invalidations, because the
vertical scrollbar never shows up. I assume that, previously, it was created and
then eventually hidden in a later layout, but now the earlier layout does not
create it either. Does that make sense?

[skobes, 2016-10-20 17:39:18]

On 2016/10/20 17:37:22, Eric Seckler wrote:
> To me, it looks like we are actually seeing fewer invalidations, because the
> vertical scrollbar never shows up. I assume that, previously, it was created
and
> then eventually hidden in a later layout, but now the earlier layout does not
> create it either. Does that make sense?

Ah ok.  Fine to rebaseline then.

[Eric Seckler, 2016-10-21 11:33:17]

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

[commit-bot: I haz the power, 2016-10-21 11:33:30]

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

[Eric Seckler, 2016-10-21 12:20:49]

On 2016/10/20 17:39:18, skobes wrote:
> On 2016/10/20 17:37:22, Eric Seckler wrote:
> > To me, it looks like we are actually seeing fewer invalidations, because the
> > vertical scrollbar never shows up. I assume that, previously, it was created
> and
> > then eventually hidden in a later layout, but now the earlier layout does
not
> > create it either. Does that make sense?
> 
> Ah ok.  Fine to rebaseline then.

Done.

[commit-bot: I haz the power, 2016-10-21 12:56:32]

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

[commit-bot: I haz the power, 2016-10-21 12:56:33]

Dry run: This issue passed the CQ dry run.

[skobes, 2016-10-21 15:56:14]

lgtm

[Eric Seckler, 2016-10-21 16:05:16]

The CQ bit was checked by eseckler@chromium.org

[commit-bot: I haz the power, 2016-10-21 16:05:51]

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, 2016-10-21 16:23:48]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-10-21 16:35:07]

Description was changed from

==========
Refactor PLSA scrollbar existence calculation.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Refactor PLSA scrollbar existence calculation.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/bf81e57ced894452410821516aa5eb94f0eb3d97
Cr-Commit-Position: refs/heads/master@{#426807}
==========

[commit-bot: I haz the power, 2016-10-21 16:35:08]

Patchset 4 (id:??) landed as
https://crrev.com/bf81e57ced894452410821516aa5eb94f0eb3d97
Cr-Commit-Position: refs/heads/master@{#426807}