Closed Bug 1448247 Opened 6 years ago Closed 6 years ago

IDBRequest can be ported to WorkerRef

Categories

(Core :: Storage: IndexedDB, enhancement, P2)

58 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: baku, Assigned: baku)

Details

Attachments

(1 file, 1 obsolete file)

      No description provided.
Attached patch patch (obsolete) — Splinter Review
Attachment #8961666 - Flags: review?(jvarga)
Comment on attachment 8961666 [details] [diff] [review]
patch

Review of attachment 8961666 [details] [diff] [review]:
-----------------------------------------------------------------

The WorkerRef stuff is very nice!

::: dom/indexedDB/IDBFactory.cpp
@@ +785,5 @@
>    }
>  
>    MOZ_ASSERT(request);
>  
> +  if (!request->IsInitialized()) {

Why is this needed ?

Should the assertion above be converted to:
if (NS_WARN_IF(!request)) {
  IDB_REPORT_INTERNAL_ERR();
  aRv.Throw(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
  return nullptr;
}
Priority: -- → P2
> Should the assertion above be converted to:
> if (NS_WARN_IF(!request)) {
>   IDB_REPORT_INTERNAL_ERR();
>   aRv.Throw(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
>   return nullptr;
> }

Think about this worker code:

self.close();
var idbRequest = idb.someOperation();
idbRequest.onsuccess = ...

after self.close() the worker goes in Closing state. With WorkerHolder, we were able to hold the worker until its state is <= Canceling. (note that Canceling is >= Closing). But WorkerRef cannot be created if the worker shutdown is already started.
This is the reason why I need to continue the operation also when WorkerRef creation failed.
Attached patch idb.patchSplinter Review
Attachment #8961666 - Attachment is obsolete: true
Attachment #8961666 - Flags: review?(jvarga)
Attachment #8969173 - Flags: review?(jvarga)
The new version of WorkerRef works much better when the worker is closing. The patch is smaller than before.
Comment on attachment 8969173 [details] [diff] [review]
idb.patch

Review of attachment 8969173 [details] [diff] [review]:
-----------------------------------------------------------------

Wow, this is really nice and simple!
Attachment #8969173 - Flags: review?(jvarga) → review+
https://hg.mozilla.org/mozilla-central/rev/14cd9f1d6365
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.