Bug 79625 - Move PointerLock APIs from Navigator.idl to NavigatorPointerLock.idl
Summary: Move PointerLock APIs from Navigator.idl to NavigatorPointerLock.idl
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 79327
  Show dependency tree / graph
 
Reported: 2012-02-26 22:41 PST by Kentaro Hara
Modified: 2012-02-26 23:28 PST (History)
4 users (show)

See Also:

Attachments
Patch (12.49 KB, patch)
2012-02-26 22:45 PST, Kentaro Hara 		abarth: review-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2012-02-26 22:41:10 PST 
For WebKit modularization, we can move PointerLock APIs from Navigator.idl to NavigatorPointerLock.idl.
Comment 1 Kentaro Hara 2012-02-26 22:45:56 PST 
Created attachment 128956 [details]
Patch
Comment 2 Adam Barth 2012-02-26 22:59:35 PST 
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.
Comment 3 Adam Barth 2012-02-26 23:00:18 PST 
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.
Comment 4 Adam Barth 2012-02-26 23:01:11 PST 
Check out NavigatorGamepad for an example of how to create an OwnPtr'ed container class for a RefCounted object that hangs off of Navigator.
Comment 5 Kentaro Hara 2012-02-26 23:28:40 PST 
(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.