Improve window.closed fidelity.

Improve window.closed fidelity.

Have window.close() mark the window object as closing, so that we can
return a more precise result for window.closed immediately upon return.

Otherwise window.closed would only transition to |true| after the deferred
closing operation has gone ahead. This exposes internal details and timing
to scripts, which is preferably avoided.

Spec-wise, for "close a browsing context",

 https://html.spec.whatwg.org/#close-a-browsing-context

window.closed will transition to |true| at the start of step 3, with
unloading&discarding (steps 3-5) being executed asynchronously (when
the deferred close operation runs.) This implies that window.closed
will return true before |pagehide| and |unload| events have been
dispatched as part of step 3 (but _after_ |beforeunload| dispatching
and handling, step 2.) This is not correct with the spec, it should
only happen once step 5 has run, but the behavior aligns with Gecko.
It is thought to be quite unlikely that unload handlers depend on
window.closed being |false| rather than |true| when run.

R=haraken,mkwst
BUG=472379
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193255

[sof, 2015-04-05 14:27:39]

sigbjornf@opera.com changed reviewers:
+ haraken@chromium.org, japhet@chromium.org, mkwst@chromium.org

[sof, 2015-04-05 14:27:39]

please take a look.

[haraken, 2015-04-05 15:06:36]

LGTM, but I want to have one more reviewer since this will involve a web-exposed
behavior change.

https://codereview.chromium.org/1053073002/diff/40001/Source/core/frame/DOMWi...
File Source/core/frame/DOMWindow.cpp (right):

https://codereview.chromium.org/1053073002/diff/40001/Source/core/frame/DOMWi...
Source/core/frame/DOMWindow.cpp:44: return m_windowIsClosing || !frame() ||
!frame()->host();

Do we still need the '!frame() || !frame()->host()' part?

[sof, 2015-04-05 15:09:42]

https://codereview.chromium.org/1053073002/diff/40001/Source/core/frame/DOMWi...
File Source/core/frame/DOMWindow.cpp (right):

https://codereview.chromium.org/1053073002/diff/40001/Source/core/frame/DOMWi...
Source/core/frame/DOMWindow.cpp:44: return m_windowIsClosing || !frame() ||
!frame()->host();
On 2015/04/05 15:06:36, haraken wrote:
> 
> Do we still need the '!frame() || !frame()->host()' part?

Yes, windows.close() is just the programmatic way to discard a browsing context.
(closed == true when that context is discarded.)

[sof, 2015-04-05 15:10:02]

On 2015/04/05 15:09:42, sof wrote:
>
https://codereview.chromium.org/1053073002/diff/40001/Source/core/frame/DOMWi...
> File Source/core/frame/DOMWindow.cpp (right):
> 
>
https://codereview.chromium.org/1053073002/diff/40001/Source/core/frame/DOMWi...
> Source/core/frame/DOMWindow.cpp:44: return m_windowIsClosing || !frame() ||
> !frame()->host();
> On 2015/04/05 15:06:36, haraken wrote:
> > 
> > Do we still need the '!frame() || !frame()->host()' part?
> 
> Yes, windows.close() is just the programmatic way to discard a browsing
context.
> (closed == true when that context is discarded.)

eh, window.close() :)

[sof, 2015-04-05 15:20:50]

This also aligns with Presto, but breaks ranks with Trident in not immediately
transitioning to true for window.closed. I don't think ToT's IE compatibility is
intentional (nor exact.)

[Mike West, 2015-04-07 06:49:11]

LGTM. I don't think we need to be terribly concerned with Trident compatibility
for this particular case. Decoupling the publicly visible behavior from the
internal details is worth the tiny shift.

[sof, 2015-04-07 07:12:36]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-04-07 07:12:44]

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

[commit-bot: I haz the power, 2015-04-07 08:34:36]

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