24
edits
(→MIME service: minor text fix) |
(→Phase 1: add notes about cycle collecting) |
||
| Line 67: | Line 67: | ||
This is a special case for adding an event listener. While one may register without any permissions, one cannot receive the events unless one is permitted (otherwise the event is dropped). Presently we overload DOMEventTargetHelper::AddEventListener to trigger this permission check, which has several variations. We should switch to overloading DOMEventTargetHelper::EventListenerWasAdded which is meant for this purpose in derived classes and is triggered no matter how the listener is added. | This is a special case for adding an event listener. While one may register without any permissions, one cannot receive the events unless one is permitted (otherwise the event is dropped). Presently we overload DOMEventTargetHelper::AddEventListener to trigger this permission check, which has several variations. We should switch to overloading DOMEventTargetHelper::EventListenerWasAdded which is meant for this purpose in derived classes and is triggered no matter how the listener is added. | ||
== Cycle collected reference counting == | |||
nsDOMDeviceStorage, nsDOMDeviceStorageCursor, DOMRequest and DeviceStorageRequest are all cycle collected objects. The reference counting for cycle collected objects is not threadsafe (and this is unlikely to change). As such great care must be used when moving references across thread boundaries, in addition to the fact we must guarantee they are freed in the owning thread context. | |||
The only class that can potentially change is DeviceStorageRequest. The cycle collection dependencies are from subclassing nsIContentPermissionRequest and holding DOMRequest, nsPIDOMWindow, and Blob references. The permission changes above will remove nsIContentPermissionRequest and the nsPIDOMWindow reference. The Blob reference may be converted into the threadsafe BlobImpl reference (which we already do later in the process). DOMRequest is more problematic. | |||
Today the code carefully moves the DOMRequest from object to object with use of .forget(). If used exclusively until we post the result on the owning thread, this will prevent modifying the reference count off the owning thread. In addition to auditing the reference counting, we would also need to use NS_ProxyRelease in all destructors of the holding classes, as well as hold a reference to the owning thread, to guarantee it is freed on the correct thread if for whatever reason the DOMRequest reference was not passed off via .forget(). | |||
The alternative is centralize all references to DOM objects in a threadsafe refcounting request manager. The manager would supply the rest of the module with opaque handles identifying the DOMRequest objects. When any other object needs to interact with the DOM objects, the manager would marshal access appropriately. For posting success/error responses, it would post to the owning thread with the desired value, do any necessary JS transforms, trigger the appropriate action (FireSuccess/FireError/FireDone) and if complete, release its reference to the DOM object. The lifecycle of a single class, which is independent of the specific operations being performed, would be much easier to manage and guarantee correctness. | |||
== Observers == | == Observers == | ||
edits