Avoid false-positives of paint offset change detection (method 2)

Avoid false-positives of paint offset change detection (method 2)

For now for discussion only, for comparison to https://codereview.chromium.org/2695593005/.

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

[Xianzhu, 2017-02-16 01:34:03]

Description was changed from

==========
Avoid false-positives of paint offset change detection (method 2)

BUG=685179
==========

to

==========
Avoid false-positives of paint offset change detection (method 2)

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

[Xianzhu, 2017-02-16 04:45:20]

wangxianzhu@chromium.org changed reviewers:
+ pdr@chromium.org

[Xianzhu, 2017-02-16 04:45:21]

Re: https://codereview.chromium.org/2695593005/#msg43

> What about when https://codereview.chromium.org/2698123002/ and
> https://codereview.chromium.org/2694193004/ are combined? I think we can avoid
> calculating the paint offset twice. updatePaintOffset would just need to call
> "object.setNeedsPaintPropertyUpdate()". The benefit is that we are still able
to
> catch under-invalidation of the paint offset translation.


Now this has https://codereview.chromium.org/2694193004/ combined in.

We still need to call calculatePaintOffsetTranslation() twice:

- In updateContextForLocation():
  
  if (calculatePaintOffsetTranslation() && paintOffsetTranslation will change)
    setNeedsPaintPropertyUpdate();

- In updatePaintOffsetTranslation():
  
  calculatePaintOffsetTranslation();
  if (needsPaintPropertyUpdate() || ...)
    reallyUpdatePaintOffsetTranslation()

Another con is that when we only need to update paintOffsetTranslation(), we
will also update all other unchanged paint properties, and may hide actual
under-invalidation of the other properties.

===========================================

In contrast, https://codereview.chromium.org/2695593005/ just unconditionally
update paintOffsetTranslation directly.

It has cons:
1. it makes updatePaintOffsetTranslation() different from other updateXXX
functions();
2. it hides under-invalidation when requirement of paint offset translation
changes.

However, I would argue about these cons:
con1. A comment can make it clear why and how updatePaintOffsetTranslation() is
different;
con2. Though it hides one case of under-invalidation, because we update
unconditionally, the under-invalidation won't matter, and we already accept
"under-invalidation of paint offset changes and paintOffsetTranslation value
changes". It can also avoid hiding under-invalidations of other properties.

And it has pros:
1. The code is a bit shorter.
2. It has better performance because
  - it calculates paintOffsetTranslation only once;
  - it won't cause update of other unchanged properties.

Previously because updateContextForObjectLocation() was public and was called
from PrePaintTreeWalk separately, it was good to separate all paint property
updates from it. Now it's no longer public and I think making it and
updatePaintOffsetTranslation() special should not be bad.

[Xianzhu, 2017-02-16 04:47:24]

Description was changed from

==========
Avoid false-positives of paint offset change detection (method 2)

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

to

==========
Avoid false-positives of paint offset change detection (method 2)

For now for discussion only, for comparison to
https://codereview.chromium.org/2695593005/.

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