[webkit-reviews] review granted: [Bug 74067] Unify willMoveToNewOwnerDocument() and didMoveToNewOwnerDocument() for better performance : [Attachment 120524] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted MORITA Hajime <morrita at google.com>'s
request for review:
Bug 74067: Unify willMoveToNewOwnerDocument() and didMoveToNewOwnerDocument()
for better performance
https://bugs.webkit.org/show_bug.cgi?id=74067

Attachment 120524: Patch
https://bugs.webkit.org/attachment.cgi?id=120524&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=120524&action=review


Please fix the one thing I mentioned; otherwise this is ready to go.

> Source/WebCore/html/HTMLMediaElement.cpp:288
> +    if (m_readyState < HAVE_CURRENT_DATA && m_shouldDelayLoadEvent) {

We do not want to check m_readyState < HAVE_CURRENT_DATA here. If this somehow
gets out of sync with m_shouldDelayLoadEvent it could result in us leaving a
document with the wrong load event delay count. This logic should solely be
based on the value of m_shouldDelayLoadEvent. The old code needed to check
m_readyState because of a flaw in how the old code worked that is now fixed by
your new function.