Early-out from the prepaint tree walk

Early-out from the prepaint tree walk

This patch stops the prepaint tree walk when no paint invalidation or
paint property updates are needed.

The three major changes are:
1) PaintPropertyTreeBuilder now detects when properties are added or
removed and forces a paint property update for all descendants. This
is needed because tree structure changes can cause descendant objects
to reference new/different ancestors.
2) When stopping the prepaint tree walk due to throttled frames, paint
invalidation and property update flags are no longer cleared. This
allows subsequent prepaint tree walks to correctly invalidate subframes
when they unthrottle.
3) PrePaintTreeWalk::walk(LayoutObject&...) exits early when possible.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-Vk/view

Performance results on blink_perf.Paint[1]:
  color-changes.html: -3.4%
  large-table-background-change-with-invisible-collapsed-borders.html: -19.2%
  large-table-background-change-with-visible-collapsed-borders.html: -3.0%
  large-table-collapsed-border-change.html: -0.4%
  large-table-collapsed-border-change-with-backgrounds.html: -6.8%
  large-table-collapsed-border-change-with-text.html: -17.2%
  large-table-repaint.html: -6.0%
  paint-offset-changes.html: -4.7%
  transform-changes.html: -2.7%

[1] tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release --pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

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

Committed: https://crrev.com/6afc534ee0a973a25230fc5f6998ecfdbce781ee
Cr-Commit-Position: refs/heads/master@{#435681}

[pdr., 2016-11-29 08:37:36]

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

[commit-bot: I haz the power, 2016-11-29 08:38:08]

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-11-29 09:13:52]

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

[commit-bot: I haz the power, 2016-11-29 09:13:53]

Dry run: Try jobs failed on following builders:
  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...)

[pdr., 2016-11-30 07:32:58]

Description was changed from

==========
Prune the prepaint tree walk

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

to

==========
Early-out from the prepaint tree walk

This patch stops the prepaint tree walk when no paint invalidation or
paint property update is needed.

The three major changes are:
1) PaintPropertyTreeBuilder now detects when properties are added or removed
and forces a paint property update for all descendants. This is needed because
tree structure changes can cause descendant objects to reference
new/different ancestors.
2) When stopping the prepaint tree walk due to throttled frames, paint
invalidation and property update flags are no longer cleared. This allows
subsequent prepaint tree walks to correctly invalidate subframes when
they unthrottle.
3) PrePaintTreeWalk::walk(LayoutObject&...) stops the treewalk if possible.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-...

Performance results on blink_perf.Paint[1]:
  color-changes.html: -3.4%
  large-table-background-change-with-invisible-collapsed-borders.html: -19.2%
  large-table-background-change-with-visible-collapsed-borders.html: -3.0%
  large-table-collapsed-border-change.html: -0.4%
  large-table-collapsed-border-change-with-backgrounds.html: -6.8%
  large-table-collapsed-border-change-with-text.html: -17.2%
  large-table-repaint.html: -6.0%
  paint-offset-changes.html: -4.7%
  transform-changes.html: -2.7%

[1] tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release
--pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

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

[pdr., 2016-11-30 07:32:58]

pdr@chromium.org changed reviewers:
+ chrishtr@chromium.org, trchen@chromium.org, wangxianzhu@chromium.org

[pdr., 2016-11-30 07:41:36]

Description was changed from

==========
Early-out from the prepaint tree walk

This patch stops the prepaint tree walk when no paint invalidation or
paint property update is needed.

The three major changes are:
1) PaintPropertyTreeBuilder now detects when properties are added or removed
and forces a paint property update for all descendants. This is needed because
tree structure changes can cause descendant objects to reference
new/different ancestors.
2) When stopping the prepaint tree walk due to throttled frames, paint
invalidation and property update flags are no longer cleared. This allows
subsequent prepaint tree walks to correctly invalidate subframes when
they unthrottle.
3) PrePaintTreeWalk::walk(LayoutObject&...) stops the treewalk if possible.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-...

Performance results on blink_perf.Paint[1]:
  color-changes.html: -3.4%
  large-table-background-change-with-invisible-collapsed-borders.html: -19.2%
  large-table-background-change-with-visible-collapsed-borders.html: -3.0%
  large-table-collapsed-border-change.html: -0.4%
  large-table-collapsed-border-change-with-backgrounds.html: -6.8%
  large-table-collapsed-border-change-with-text.html: -17.2%
  large-table-repaint.html: -6.0%
  paint-offset-changes.html: -4.7%
  transform-changes.html: -2.7%

[1] tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release
--pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

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

to

==========
Early-out from the prepaint tree walk

This patch stops the prepaint tree walk when no paint invalidation or
paint property updates are needed.

The three major changes are:
1) PaintPropertyTreeBuilder now detects when properties are added or
removed and forces a paint property update for all descendants. This
is needed because tree structure changes can cause descendant objects
to reference new/different ancestors.
2) When stopping the prepaint tree walk due to throttled frames, paint
invalidation and property update flags are no longer cleared. This
allows subsequent prepaint tree walks to correctly invalidate subframes
when they unthrottle.
3) PrePaintTreeWalk::walk(LayoutObject&...) exits early when possible.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-...

Performance results on blink_perf.Paint[1]:
  color-changes.html: -3.4%
  large-table-background-change-with-invisible-collapsed-borders.html: -19.2%
  large-table-background-change-with-visible-collapsed-borders.html: -3.0%
  large-table-collapsed-border-change.html: -0.4%
  large-table-collapsed-border-change-with-backgrounds.html: -6.8%
  large-table-collapsed-border-change-with-text.html: -17.2%
  large-table-repaint.html: -6.0%
  paint-offset-changes.html: -4.7%
  transform-changes.html: -2.7%

[1] tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release
--pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

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

[pdr., 2016-11-30 07:42:28]

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

[commit-bot: I haz the power, 2016-11-30 07:43:35]

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-11-30 08:16:28]

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

[commit-bot: I haz the power, 2016-11-30 08:16:29]

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

[pdr., 2016-11-30 19:17:09]

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

[commit-bot: I haz the power, 2016-11-30 19:17:36]

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-11-30 19:56:42]

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

[commit-bot: I haz the power, 2016-11-30 19:56:43]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[pdr., 2016-11-30 22:27:01]

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

[commit-bot: I haz the power, 2016-11-30 22:27:22]

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

[Xianzhu, 2016-11-30 23:59:19]

lgtm

[commit-bot: I haz the power, 2016-12-01 00:07:33]

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

[commit-bot: I haz the power, 2016-12-01 00:07:34]

Dry run: This issue passed the CQ dry run.

[chrishtr, 2016-12-01 02:36:35]

https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:2737: //
|paintInvalidationParent()| is used to ensure we continue marking across
I don't think this comment is necessary.

https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:2743: void
LayoutObject::setDescendantNeedsPaintPropertyUpdate() {
*markAncestorChain* is how other bit-setting methods are named for other tree
walks, like layout and
descendant-dependent PaintLayer flags.

https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp (right):

https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:134:
context.forceSubtreeUpdate |= updatePreTranslation(
Did you consider passing context.forceSubtreeUpdate as a bool& parameter to each
of these setters or clearers?

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/FindPropertiesNeedingUpdate.h (right):

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/FindPropertiesNeedingUpdate.h:31:
m_hadForceSubtreeUpdate(context.forceSubtreeUpdate) {
Can you also use this field to check that forceSubtree was set to true due to
properties being set?

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp (right):

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:898:
.setNeedsPaintPropertyUpdateWithoutMarkingAncestors();
This is kind of an odd way to do forced subtree update (you're setting the bits
just ahead of checking them, right?).
Is this better than just checking (hasBit | forced)?

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h (right):

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h:25: // Throttled
rendering (see: FrameView::shouldThrottleRendering()) can prevent
Please add a unittest for the throttling early-out.

[pdr., 2016-12-01 09:35:35]

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/FindPropertiesNeedingUpdate.h (right):

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/FindPropertiesNeedingUpdate.h:31:
m_hadForceSubtreeUpdate(context.forceSubtreeUpdate) {
On 2016/12/01 at 02:36:35, chrishtr wrote:
> Can you also use this field to check that forceSubtree was set to true due to
properties being set?

This class catches underinvalidation by checking that both property pointers and
property values do not change for objects not needing an update. If forceSubtree
were set to true while this checker is in scope, it would mean that property
pointers changed which this class should already check for.

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp (right):

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:898:
.setNeedsPaintPropertyUpdateWithoutMarkingAncestors();
On 2016/12/01 at 02:36:35, chrishtr wrote:
> This is kind of an odd way to do forced subtree update (you're setting the
bits just ahead of checking them, right?).
> Is this better than just checking (hasBit | forced)?

This is a great idea. Done.

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h (right):

https://codereview.chromium.org/2539693002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h:25: // Throttled
rendering (see: FrameView::shouldThrottleRendering()) can prevent
On 2016/12/01 at 02:36:35, chrishtr wrote:
> Please add a unittest for the throttling early-out.

This was tested by fast/replaced/no-focus-ring-iframe.html but a unittest is
much better. Done.

[pdr., 2016-12-01 09:46:53]

On 2016/12/01 at 02:36:35, chrishtr wrote:
>
https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/LayoutObject.cpp:2743: void
LayoutObject::setDescendantNeedsPaintPropertyUpdate() {
> *markAncestorChain* is how other bit-setting methods are named for other tree
walks, like layout and
> descendant-dependent PaintLayer flags.

In the current patch I was able to remove the need for LayoutObject users to
know about marking ancestors, so there's no more
setDescendantNeedsPaintPropertyUpdate. I also switched some terms to use
"marking" while including context about what it means. Can you take another
look?

> 
>
https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
(right):
> 
>
https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:134:
context.forceSubtreeUpdate |= updatePreTranslation(
> Did you consider passing context.forceSubtreeUpdate as a bool& parameter to
each of these setters or clearers?

I tried a bunch of variations and none of them are fantastic :/ Sadly, clang on
windows didn't like my preferred enum approach.

A downside of the bool& approach is that the update functions have to implicitly
know to only set the bool in one direction. For example:
void createOrUpdate(bool& forceSubtreeUpdate) {
  if (!m_foo) {
    m_foo = new foo();
    forceSubtreeUpdate = true;
  } else {
    m_foo.update(...);
    forceSubtreeUpdate = false; // bug!
  }
}
This function looks correct but has a subtle bug because we should never set
forceSubtreeUpdate to false, only to true. Do you know of a way to make this
pattern less error-prone?

[pdr., 2016-12-01 09:50:08]

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

[commit-bot: I haz the power, 2016-12-01 09:50:19]

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-12-01 13:31:55]

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

[commit-bot: I haz the power, 2016-12-01 13:31:56]

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_...)

[chrishtr, 2016-12-01 15:16:33]

On 2016/12/01 at 09:46:53, pdr wrote:
> On 2016/12/01 at 02:36:35, chrishtr wrote:
> >
https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/layout/LayoutObject.cpp:2743: void
LayoutObject::setDescendantNeedsPaintPropertyUpdate() {
> > *markAncestorChain* is how other bit-setting methods are named for other
tree walks, like layout and
> > descendant-dependent PaintLayer flags.
> 
> In the current patch I was able to remove the need for LayoutObject users to
know about marking ancestors, so there's no more
setDescendantNeedsPaintPropertyUpdate. I also switched some terms to use
"marking" while including context about what it means. Can you take another
look?
> 
> > 
> >
https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
(right):
> > 
> >
https://codereview.chromium.org/2539693002/diff/80001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:134:
context.forceSubtreeUpdate |= updatePreTranslation(
> > Did you consider passing context.forceSubtreeUpdate as a bool& parameter to
each of these setters or clearers?
> 
> I tried a bunch of variations and none of them are fantastic :/ Sadly, clang
on windows didn't like my preferred enum approach.
> 
> A downside of the bool& approach is that the update functions have to
implicitly know to only set the bool in one direction. For example:
> void createOrUpdate(bool& forceSubtreeUpdate) {
>   if (!m_foo) {
>     m_foo = new foo();
>     forceSubtreeUpdate = true;
>   } else {
>     m_foo.update(...);
>     forceSubtreeUpdate = false; // bug!
>   }
> }
> This function looks correct but has a subtle bug because we should never set
forceSubtreeUpdate to false, only to true. Do you know of a way to make this
pattern less error-prone?

I've got no further ideas either..

[chrishtr, 2016-12-01 15:16:51]

lgtm

[pdr., 2016-12-01 19:52:11]

On 2016/12/01 at 15:16:51, chrishtr wrote:
> lgtm

Thanks!

In we go

[pdr., 2016-12-01 19:52:16]

The CQ bit was checked by pdr@chromium.org

[pdr., 2016-12-01 19:52:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2539693002/#ps140001
(title: "Address chrishtrs comments")

[commit-bot: I haz the power, 2016-12-01 19:52:44]

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-12-01 19:59:01]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1480621936061260,
"parent_rev": "5a6c56539f6aab2b2e6662930ee5543aa57224e7", "commit_rev":
"e30d590283e06746f3d7fd1cafd4881170354239"}

[commit-bot: I haz the power, 2016-12-01 19:59:30]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-12-01 20:02:31]

Description was changed from

==========
Early-out from the prepaint tree walk

This patch stops the prepaint tree walk when no paint invalidation or
paint property updates are needed.

The three major changes are:
1) PaintPropertyTreeBuilder now detects when properties are added or
removed and forces a paint property update for all descendants. This
is needed because tree structure changes can cause descendant objects
to reference new/different ancestors.
2) When stopping the prepaint tree walk due to throttled frames, paint
invalidation and property update flags are no longer cleared. This
allows subsequent prepaint tree walks to correctly invalidate subframes
when they unthrottle.
3) PrePaintTreeWalk::walk(LayoutObject&...) exits early when possible.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-...

Performance results on blink_perf.Paint[1]:
  color-changes.html: -3.4%
  large-table-background-change-with-invisible-collapsed-borders.html: -19.2%
  large-table-background-change-with-visible-collapsed-borders.html: -3.0%
  large-table-collapsed-border-change.html: -0.4%
  large-table-collapsed-border-change-with-backgrounds.html: -6.8%
  large-table-collapsed-border-change-with-text.html: -17.2%
  large-table-repaint.html: -6.0%
  paint-offset-changes.html: -4.7%
  transform-changes.html: -2.7%

[1] tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release
--pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

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

to

==========
Early-out from the prepaint tree walk

This patch stops the prepaint tree walk when no paint invalidation or
paint property updates are needed.

The three major changes are:
1) PaintPropertyTreeBuilder now detects when properties are added or
removed and forces a paint property update for all descendants. This
is needed because tree structure changes can cause descendant objects
to reference new/different ancestors.
2) When stopping the prepaint tree walk due to throttled frames, paint
invalidation and property update flags are no longer cleared. This
allows subsequent prepaint tree walks to correctly invalidate subframes
when they unthrottle.
3) PrePaintTreeWalk::walk(LayoutObject&...) exits early when possible.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-...

Performance results on blink_perf.Paint[1]:
  color-changes.html: -3.4%
  large-table-background-change-with-invisible-collapsed-borders.html: -19.2%
  large-table-background-change-with-visible-collapsed-borders.html: -3.0%
  large-table-collapsed-border-change.html: -0.4%
  large-table-collapsed-border-change-with-backgrounds.html: -6.8%
  large-table-collapsed-border-change-with-text.html: -17.2%
  large-table-repaint.html: -6.0%
  paint-offset-changes.html: -4.7%
  transform-changes.html: -2.7%

[1] tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release
--pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

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

Committed: https://crrev.com/6afc534ee0a973a25230fc5f6998ecfdbce781ee
Cr-Commit-Position: refs/heads/master@{#435681}
==========

[commit-bot: I haz the power, 2016-12-01 20:02:33]

Patchset 8 (id:??) landed as
https://crrev.com/6afc534ee0a973a25230fc5f6998ecfdbce781ee
Cr-Commit-Position: refs/heads/master@{#435681}