[webkit-reviews] review denied: [Bug 22562] REGRESSION (r37971): events not firing after going back in back/forward cache : [Attachment 25981] Proposed patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Cameron Zwarich (cpst)
<cwzwarich at uwaterloo.ca>'s request for review:
Bug 22562: REGRESSION (r37971): events not firing after going back in
back/forward cache
https://bugs.webkit.org/show_bug.cgi?id=22562

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

------- Additional Comments from Darin Adler <darin at apple.com>
This is not quite the right place for this. If you look in FrameLoader::open
you'll see that before calling CachedPage::restore, there are a couple calls on
m_frame that change fields in the domWindow. So it's too late to change the
DOMWindow after that code has already run. We probably should set the DOMWindow
at almost exactly the same time we set the document. I don't really have an
opinion about what code should go inside CachedPage::restore vs. what code goes
in FrameLoader::open -- CachedPage::restore is only called from that one
function, and the two work together without a clear division of
responsibilities -- but clearly there's an order dependency that we have to get
right.

Also, if you add a function the name should be setDOMWindow, not setDomWindow.

I think it's important in the longer term to have some regression tests for
this, as you have mentioned before.