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:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

39879, 40054

Attachments:

Description	Flags
Patch	jorlow: review-

Andrei Popescu

Reported 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.

Attachments
Patch (3.20 KB, patch) 2010-06-02 11:44 PDT, Andrei Popescu	jorlow: review-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Andrei Popescu

Comment 1 2010-06-02 11:44:55 PDT

Created attachment 57675 [details] Patch

Jeremy Orlow

Comment 2 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

Jeremy Orlow

Comment 3 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.

Steve Block

Comment 4 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.

Jeremy Orlow

Comment 5 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.

Steve Block

Comment 6 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.

Andrei Popescu

Comment 7 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 :)

Note You need to log in before you can comment on or make changes to this bug.