[webkit-reviews] review granted: [Bug 194360] HTMLMediaElement registers wrong ScriptExecutionContext with its ActiveDOMObject parent class : [Attachment 361329] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 194360: HTMLMediaElement registers wrong ScriptExecutionContext with its
ActiveDOMObject parent class
https://bugs.webkit.org/show_bug.cgi?id=194360

Attachment 361329: Patch

https://bugs.webkit.org/attachment.cgi?id=361329&action=review




--- Comment #6 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 361329
  --> https://bugs.webkit.org/attachment.cgi?id=361329
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361329&action=review

r=me

> Source/WebCore/dom/ActiveDOMObject.h:118
>      explicit ActiveDOMObject(ScriptExecutionContext*);

Maybe we should, in a follow-up patch, remove this constructor entirely, since
it's still an opportunity for our caller to forget to call contextDocument().

Also, as we discussed in person, it would be nice to refactor Document so that
it has a ScriptExecutionContext accessor, rather than inheriting from
ScriptExecutionContext. Inheriting from ScriptExecutionContext is wrong because
DOMImplementation.createDocument() returns a document that is not a
ScriptExecutionContext and must not be used as one, and also because the
document is a property of the window/frame/ScriptExecutionContext, and is not
the ScriptExecutionContext itself.