Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrance of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is processing.

This CL checks reentrance of FrameView::updateLifecyclePhasesInternal(),
and crash with NOTREACHED() if DCHECK is enabled or early return
otherwise.

BUG=590856, 621360
Committed: https://crrev.com/66f753c15a71e28594ec9502b6d1f2b9c9d14dba
Cr-Commit-Position: refs/heads/master@{#416712}

[Xianzhu, 2016-09-02 17:01:15]

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

[commit-bot: I haz the power, 2016-09-02 17:01:45]

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

[Xianzhu, 2016-09-02 17:07:09]

Description was changed from

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

BUG=590856,621360
==========

to

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrence of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is being processed.

This CL add a DCHECK to check reentrance of
FrameView::updateLifecyclePhasesInternal(), and early return if
DCHECK is not enabled.

BUG=590856,621360
==========

[Xianzhu, 2016-09-02 17:07:37]

wangxianzhu@chromium.org changed reviewers:
+ eae@chromium.org, szager@google.com, wkorman@chromium.org

[Xianzhu, 2016-09-02 17:07:54]

Description was changed from

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrence of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is being processed.

This CL add a DCHECK to check reentrance of
FrameView::updateLifecyclePhasesInternal(), and early return if
DCHECK is not enabled.

BUG=590856,621360
==========

to

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrence of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is being processed.

This CL adds a DCHECK to check reentrance of
FrameView::updateLifecyclePhasesInternal(), and an early return if
DCHECK is not enabled.

BUG=590856,621360
==========

[wkorman, 2016-09-02 17:39:37]

lgtm

nit: update CL description to reflect use of NOTREACHED() vs DCHECK. Does this
pass a debug trybot? Interested to see.

[Xianzhu, 2016-09-02 18:04:36]

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

[Xianzhu, 2016-09-02 18:05:35]

Description was changed from

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrence of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is being processed.

This CL adds a DCHECK to check reentrance of
FrameView::updateLifecyclePhasesInternal(), and an early return if
DCHECK is not enabled.

BUG=590856,621360
==========

to

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrence of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is being processed.

This CL checks reentrance of FrameView::updateLifecyclePhasesInternal(),
and crash with NOTREACHED() if DCHECK is enabled or early return
otherwise.

BUG=590856,621360
==========

[commit-bot: I haz the power, 2016-09-02 18:06:09]

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

[Xianzhu, 2016-09-02 18:06:34]

Description was changed from

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrence of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is being processed.

This CL checks reentrance of FrameView::updateLifecyclePhasesInternal(),
and crash with NOTREACHED() if DCHECK is enabled or early return
otherwise.

BUG=590856,621360
==========

to

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrence of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is processing.

This CL checks reentrance of FrameView::updateLifecyclePhasesInternal(),
and crash with NOTREACHED() if DCHECK is enabled or early return
otherwise.

BUG=590856,621360
==========

[Xianzhu, 2016-09-02 18:08:06]

Description was changed from

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrence of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is processing.

This CL checks reentrance of FrameView::updateLifecyclePhasesInternal(),
and crash with NOTREACHED() if DCHECK is enabled or early return
otherwise.

BUG=590856,621360
==========

to

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrance of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is processing.

This CL checks reentrance of FrameView::updateLifecyclePhasesInternal(),
and crash with NOTREACHED() if DCHECK is enabled or early return
otherwise.

BUG=590856,621360
==========

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

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

[commit-bot: I haz the power, 2016-09-02 20:01:10]

Dry run: Exceeded global retry quota

[eae, 2016-09-05 20:28:26]

LGTM

[Xianzhu, 2016-09-06 17:52:28]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-09-06 17:53:36]

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

[Xianzhu, 2016-09-06 17:56:13]

On 2016/09/02 17:39:37, wkorman wrote:
> lgtm
> 
> nit: update CL description to reflect use of NOTREACHED() vs DCHECK.

Done.

> Does this pass a debug trybot? Interested to see.

Most of the release try bots have dcheck_always_on so they cover this CL. Tried
on dbg bots anyway.

DumpAccessibilityTreeTest.AccessibilityFrameset and
DumpAccessibilityTreeTest.AccessibilityIframeCoordinatesCrossProcess (in
content_browsertests) crashed on win and mac respectively. The crash stacks
don't show anything related to this CL. Retrying to see if they are temporary
flakiness. Will look into them if they still crash.

[commit-bot: I haz the power, 2016-09-06 20:39:43]

Description was changed from

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrance of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is processing.

This CL checks reentrance of FrameView::updateLifecyclePhasesInternal(),
and crash with NOTREACHED() if DCHECK is enabled or early return
otherwise.

BUG=590856,621360
==========

to

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrance of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is processing.

This CL checks reentrance of FrameView::updateLifecyclePhasesInternal(),
and crash with NOTREACHED() if DCHECK is enabled or early return
otherwise.

BUG=590856,621360
==========

[commit-bot: I haz the power, 2016-09-06 20:39:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-06 20:41:24]

Description was changed from

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrance of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is processing.

This CL checks reentrance of FrameView::updateLifecyclePhasesInternal(),
and crash with NOTREACHED() if DCHECK is enabled or early return
otherwise.

BUG=590856,621360
==========

to

==========
Disallow reentrance of FrameView::updateLifecyclePhasesInternal()

Reentrance of FrameView::updateLifecyclePhasesInternal() is bad.
For example, when we are invalidating paint, we call a function which
looks like a non-side-affecting getter which unexpectedly initiates
another lifecycle update which may unexpectedly delete the object
that the caller is processing.

This CL checks reentrance of FrameView::updateLifecyclePhasesInternal(),
and crash with NOTREACHED() if DCHECK is enabled or early return
otherwise.

BUG=590856,621360

Committed: https://crrev.com/66f753c15a71e28594ec9502b6d1f2b9c9d14dba
Cr-Commit-Position: refs/heads/master@{#416712}
==========

[commit-bot: I haz the power, 2016-09-06 20:41:25]

Patchset 2 (id:??) landed as
https://crrev.com/66f753c15a71e28594ec9502b6d1f2b9c9d14dba
Cr-Commit-Position: refs/heads/master@{#416712}