Revert of Revert of Issue FrameDestructionObserver::frameDestroyed() notification on detach.

Revert of Revert of Issue FrameDestructionObserver::frameDestroyed() notification on detach. (patchset #1 id:1 of https://codereview.chromium.org/896103002/)

Reason for revert:
ChromeOS bot did not cycle green at Blink r189454, so will re-land Blink r189443.

Original issue's description:
> Revert of Issue FrameDestructionObserver::frameDestroyed() notification on detach. (patchset #2 id:20001 of https://codereview.chromium.org/881683003/)
> 
> Reason for revert:
> ScreenOrientationBrowserTest.CrashTest_UseAfterDetach failing on ChromeOS b/w
> blink 189439 - 189443, suspect 189443, revert to see if ChromeOS greens.
> 
> 
> Original issue's description:
> > Issue FrameDestructionObserver::frameDestroyed() notification on detach.
> > 
> > Send this notification during detach instead of waiting until finalization,
> > which will typically proceed shortly afterwards (non-Oilpan.)
> > 
> > Doing so provides identical lifetimes for FrameDestructionObserver's
> > frame reference, with and without Oilpan. Making GC/finalization of frames
> > not be externally observable is preferable overall.
> > 
> > R=haraken,dcheng
> > BUG=446452
> > 
> > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189443
> 
> TBR=dcheng@chromium.org,haraken@chromium.org,oilpan-reviews@chromium.org,sigbjornf@opera.com
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=446452
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189454

TBR=dcheng@chromium.org,haraken@chromium.org,oilpan-reviews@chromium.org,sigbjornf@opera.com,japhet@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=446452
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189457

[Noel Gordon, 2015-02-04 06:05:10]

Created Revert of Revert of Issue FrameDestructionObserver::frameDestroyed()
notification on detach.

[commit-bot: I haz the power, 2015-02-04 06:05:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/890243003/1

[commit-bot: I haz the power, 2015-02-04 06:06:41]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189457

[haraken, 2015-02-04 07:07:46]

LGTM

[Noel Gordon, 2015-02-04 07:11:06]

On 2015/02/04 07:07:46, haraken wrote:
> LGTM

Do stare at the stack trace over on https://codereview.chromium.org/896103002
somethings not right.  Could this change race with a js callback?

[sof, 2015-02-04 07:29:10]

On 2015/02/04 07:11:06, noel gordon wrote:
> On 2015/02/04 07:07:46, haraken wrote:
> > LGTM
> 
> Do stare at the stack trace over on https://codereview.chromium.org/896103002
> somethings not right.  Could this change race with a js callback?

The ScreenOrientationController needs to be adjusted a bit -- it is a
FrameDestructionObserver, so when its frame is destructed it needs to stop
updates from the platform event controller. Similar to what the gamepad
navigator controller is doing. Not doing so brings about that frame assert, I
reckon.

IOW, the CL here is exposing that assert. I'm surprised the issue isn't fixed by
reverting it -- could you double check?

[sof, 2015-02-04 07:30:37]

On 2015/02/04 07:29:10, sof wrote:
> On 2015/02/04 07:11:06, noel gordon wrote:
> > On 2015/02/04 07:07:46, haraken wrote:
> > > LGTM
> > 
> > Do stare at the stack trace over on
https://codereview.chromium.org/896103002
> > somethings not right.  Could this change race with a js callback?
> 
> The ScreenOrientationController needs to be adjusted a bit -- it is a
> FrameDestructionObserver, so when its frame is destructed it needs to stop
> updates from the platform event controller. Similar to what the gamepad
> navigator controller is doing. Not doing so brings about that frame assert, I
> reckon.
> 
> IOW, the CL here is exposing that assert. I'm surprised the issue isn't fixed
by
> reverting it -- could you double check?

This is timing dependent, so I wouldn't expect a steady repro.

[Noel Gordon, 2015-02-04 10:20:30]

On 2015/02/04 07:30:37, sof wrote:
> On 2015/02/04 07:29:10, sof wrote:
>
> > IOW, the CL here is exposing that assert. I'm surprised the issue isn't
fixed
> by reverting it -- could you double check?
> 
> This is timing dependent, so I wouldn't expect a steady repro.

Yeah a race condition of some sort, so no sense to revert if you are happy with
your side of the change here, right?  I have to assume that to move the tree
forward.

[Noel Gordon, 2015-02-04 10:23:09]

On 2015/02/04 07:29:10, sof wrote:
> On 2015/02/04 07:11:06, noel gordon wrote:
> > On 2015/02/04 07:07:46, haraken wrote:
> > > LGTM
> > 
> > Do stare at the stack trace over on
https://codereview.chromium.org/896103002
> > somethings not right.  Could this change race with a js callback?

> The ScreenOrientationController needs to be adjusted a bit -- it is a
> FrameDestructionObserver, so when its frame is destructed it needs to stop
> updates from the platform event controller. Similar to what the gamepad
> navigator controller is doing. Not doing so brings about that frame assert, I
> reckon.

RIght sof@, good stuff, this seems suitable for a bug report.  Let me file.

[sof, 2015-02-04 10:25:10]

On 2015/02/04 10:20:30, noel gordon wrote:
> On 2015/02/04 07:30:37, sof wrote:
> > On 2015/02/04 07:29:10, sof wrote:
> >
> > > IOW, the CL here is exposing that assert. I'm surprised the issue isn't
> fixed
> > by reverting it -- could you double check?
> > 
> > This is timing dependent, so I wouldn't expect a steady repro.
> 
> Yeah a race condition of some sort, so no sense to revert if you are happy
with
> your side of the change here, right?  I have to assume that to move the tree
> forward.

Let's revert it for now, even if you see flakiness without it.

[Noel Gordon, 2015-02-04 10:37:03]

k, can you prepare the revert and I'll lgtm please.

[Noel Gordon, 2015-02-04 10:44:26]

> Right sof@, good stuff, this seems suitable for a bug report.  Let me file.

https://code.google.com/p/chromium/issues/detail?id=455161