Created attachment 128956 [details] Patch

Comment on attachment 128956 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128956&action=review > Source/WebCore/Modules/pointerlock/NavigatorPointerLock.idl:25 > + ] NavigatorPointerLock { There are a bunch of ENABLED(POINTER_LOCK) ifdefs in WebCore proper. It might be better to wait on creating a pointerlock module until we're certain we can remove them all (or at least the vast majority of them). > Source/WebCore/page/PointerLock.h:43 > -class PointerLock : public RefCounted<PointerLock>, public DOMWindowProperty { > +class PointerLock : public RefCounted<PointerLock>, public DOMWindowProperty, public NavigatorSupplement { PointerLock is RefCounted but NavigatorSupplement uses OwnPtr. This patch will create a use-after-free bug. We should make NavigatorPointerLock be the NavigatorSupplement, which can hold a RefPtr to the PointerLock object.

Note: This patch conflicts with <http://trac.webkit.org/changeset/108958>. You'll probably want to update past that revision when working on this patch.

Check out NavigatorGamepad for an example of how to create an OwnPtr'ed container class for a RefCounted object that hangs off of Navigator.

(In reply to comment #2) > (From update of attachment 128956 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=128956&action=review > > > Source/WebCore/Modules/pointerlock/NavigatorPointerLock.idl:25 > > + ] NavigatorPointerLock { > > There are a bunch of ENABLED(POINTER_LOCK) ifdefs in WebCore proper. It might be better to wait on creating a pointerlock module until we're certain we can remove them all (or at least the vast majority of them). Makes sense. As far as I see, it seems difficult to get rid of them. For now let me mark this bug as WONTFIX.