Bug 40065

Summary: Geolocation callbacks should make use of new callback generation mechanism
Product: WebKit Reporter: Steve Block <steveblock>
Component: WebCore Misc.Assignee: Steve Block <steveblock>
Status: RESOLVED FIXED    
Severity: Normal CC: andreip, ap, buildbot, dbates, jknotten, leandrogracia, steveblock
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 39879    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Steve Block 2010-06-02 08:24:53 PDT 
Geolocation callbacks should make use of new callback generation mechanism. This will remove the need for the Geolocation custom callback code. See the equivalent change for Database in http://trac.webkit.org/changeset/58801
Comment 1 Steve Block 2010-06-02 11:12:18 PDT 
Bug 40071 is adding a weak pointer wrapper around ScriptExecutionContext. This is required for Bug 39879 which fixes a Geolocation crash. We should hold off on switching to the generated callbacks until this is complete and Geolocation tests are in place to check the behavior.
Comment 2 Steve Block 2011-03-24 03:55:46 PDT 
The resolution of Bug 40071 was to not use a weak wrapper around ScriptExecutionContext, but instead to make callbacks inherit from ActiveDOMObject. The (hand written) Geolocation callbacks were updated to use this approach in http://trac.webkit.org/changeset/60840

http://trac.webkit.org/changeset/58801 updated the bindings script to automatically generate callbacks, and updated all DB callbacks to be automatically generated. The automatically generated callbacks were updated in http://trac.webkit.org/changeset/64537 to use ActiveDOMObject, similar to how the Geolocation callbacks do.

So we should now be able to switch to the automatically generated callbacks for Geolocation.

Note that there is ongoing discussion about a more fundamental refactoring of Geolocation - https://bugs.webkit.org/show_bug.cgi?id=40162 - but I think that eliminating custom bindings code still makes sense now.
Comment 3 Steve Block 2011-03-24 04:07:18 PDT 
Adding Leandro as this will be relevant to Media Streaming. See Bug 56459.
Comment 4 Steve Block 2011-04-13 03:57:52 PDT 
Created attachment 89361 [details]
Patch
Comment 5 Daniel Bates 2011-04-13 04:30:58 PDT 
Attachment 89361 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8404205
Comment 6 Build Bot 2011-04-13 05:20:30 PDT 
Attachment 89361 [details] did not build on win:
Build output: http://queues.webkit.org/results/8401286
Comment 7 Steve Block 2011-04-13 08:47:33 PDT 
Created attachment 89382 [details]
Patch
Comment 8 Eric Seidel (no email) 2011-05-23 15:15:48 PDT 
Comment on attachment 89382 [details]
Patch

Um.  I like deleting code.  So rs=me.
Comment 9 Steve Block 2011-05-23 15:36:57 PDT 
Created attachment 94505 [details]
Patch
Comment 10 Steve Block 2011-05-23 15:39:52 PDT 
Not for review.

Rebased to account for deletion of android Makefiles. Will wait for bots to cycle before submitting.
Comment 11 WebKit Commit Bot 2011-05-23 22:45:03 PDT 
Comment on attachment 94505 [details]
Patch

Attachment 94505 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8727497
Comment 12 Steve Block 2011-05-24 02:38:49 PDT 
Created attachment 94586 [details]
Patch
Comment 13 Steve Block 2011-05-24 03:54:48 PDT 
Committed r87143: <http://trac.webkit.org/changeset/87143>