Bug 40071

Summary: Objects invoking script need to know when the relevant ScriptExecutionContext has torn down.
Product: WebKit Reporter: Andrei Popescu <andreip>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: ap, jorlow, steveblock
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 39879, 40054    
Attachments:
Description Flags
Patch jorlow: review-

Description Andrei Popescu 2010-06-02 11:03:17 PDT
Objects invoking script (e.g. geolocation, indexedDB, database) need a ScriptExecutionContext instance in order to be able to invoke script (e.g. when a new geoposition is available, an object store is open, an SQL statement finishes executing). In all these cases, Javascript should be invoked in the context in which the object's method was called. However, at Javascript invocation time, the relevant ScriptExecutionContext may have been torn down. Note that the ScriptExecutionContext may be from frames other than the one that owns the object so we cannot rely on the owner frame's "disconnectFrame".  Instead, we need to if the relevant ScriptExecutionContext still exists at script invocation time. This could be achieved by adding a "weak pointer" wrapper around each ScriptExecutionContext instance. Objects that need a ScriptExecutionContext, can use the weak pointer instead.
Comment 1 Andrei Popescu 2010-06-02 11:44:55 PDT
Created attachment 57675 [details]
Patch
Comment 2 Jeremy Orlow 2010-06-02 11:50:27 PDT
Comment on attachment 57675 [details]
Patch

Steve tells me that he's soon going to land code that uses this and adds test coverage.  I know we're going to be using it on IndexedDB soon as well.  (And hopefully someone can switch WebDatabase over as well.)  So r=me
Comment 3 Jeremy Orlow 2010-06-02 12:26:45 PDT
Ahhh!  I'm an idiot.  You should be using ActiveDOMObject for this case.  Everywhere I can think of that you'd need to worry about your context going away you'd also need to worry about the page being suspended and such.
Comment 4 Steve Block 2010-06-02 12:42:44 PDT
(In reply to comment #3)
> Ahhh!  I'm an idiot.  You should be using ActiveDOMObject for this case. 
> Everywhere I can think of that you'd need to worry about your context going
> away you'd also need to worry about the page being suspended and such.
Are you sure about this? I'm not very familiar with ActiveDOMObject but I'd imagined that it only notifies an object about changes to the script context for the frame that owns the object. We need to know about the state of all the script contexts from which the object's methods were invoked and hence we must make callbacks in.
Comment 5 Jeremy Orlow 2010-06-02 13:23:21 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > Ahhh!  I'm an idiot.  You should be using ActiveDOMObject for this case. 
> > Everywhere I can think of that you'd need to worry about your context going
> > away you'd also need to worry about the page being suspended and such.
> Are you sure about this? I'm not very familiar with ActiveDOMObject but I'd imagined that it only notifies an object about changes to the script context for the frame that owns the object. We need to know about the state of all the script contexts from which the object's methods were invoked and hence we must make callbacks in.

When you create a subclass of ActiveDOMObject you need to supply the ScriptExecutionContext* to the constructor.  This is what determines which ScriptExecutionContext you watch.  If you're not sure what should implement the ActiveDOMObject in geoLocation, I'm happy to look at that with you tomorrow.  For indexedDB, the answer is IDBRequest.  Whenever any other ____Request object instantiates it, it'll pass in the current ScriptExecutionContext and everything should work out fine.  In WebSQLDatabase, the callback object itself should be an active dom object.  I imagine geolocation is similar.
Comment 6 Steve Block 2010-06-03 02:54:55 PDT
Got it - I thought you were suggesting using ActiveDOMObject for the Database/Geolocation object. Will send a patch for Geolocation to Bug 39879 soon.
Comment 7 Andrei Popescu 2010-06-03 03:10:00 PDT
(In reply to comment #3)
> Ahhh!  I'm an idiot.  You should be using ActiveDOMObject for this case.  Everywhere I can think of that you'd need to worry about your context going away you'd also need to worry about the page being suspended and such.

Heh, I totally forgot our IDBRequest class is already an ActiveDOMObject. Oh well, it happens :)