[webkit-reviews] review requested: [Bug 28716] Event listeners installed on a window object returned from window.open() don't work : [Attachment 39033] Proposed patch

[bugzilla-daemon at webkit.org]

Dmitry Titov <dimich at chromium.org> has asked  for review:
Bug 28716: Event listeners installed on a window object returned from
window.open() don't work
https://bugs.webkit.org/show_bug.cgi?id=28716

Attachment 39033: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=39033&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
The proposed fix makes WebKit behavior match both html5 spec and FF 3.5. Also
adds tests.

Now we don't remove event listeners on DOMWindow in case of transitioning from
empty temporary document (created by FrameLoader::init()) to final document.
This is accomplished by the check in FrameLoader::stopLoading that checks for
the safe transiotion (similar to the one in FrameLoader::begin(..) that
prevents erasing window properties).

Another place that was removing all the event handlers was Document::clear(),
invoked from Document::implicitOpen() which is called in 2 cases: when
transition document is replaced with loaded content (case of this bug) and when
document.open() is used from JS. In latter case we actually need to clean event
handlers - if only to avoid firing 'load' upon subsequent document.close(), or
things will fail on the web. So I've split clear() and moved clearing handlers
code to Document::open(..) and the rest to implicitOpen() and removed the
Document::clear() at all.

Also FrameLoader::closeURL() was called 2 times during regular navigation -
from transitionToCommitted and then immediately after it returns to
commitProvisionalLoad via didOpenURL. The problem with that is closeURL calls
stopLoading and that tries to fire unload again - so the old code, by blowing
all the event handlers,  was making unload fire once, so it was ok to call it
twice. Now that we don't always blow the event handlers, that would cause 2
unload events fired - so I move it out of didOpenURL.