Bug 701634 (AsyncIDB)

IndexedDB: Support database access from worker threads

RESOLVED FIXED in mozilla37

Status

()

Core
DOM: IndexedDB
RESOLVED FIXED
6 years ago
6 months ago

People

(Reporter: blizzard, Assigned: Ben Turner (not reading bugmail, use the needinfo flag!))

Tracking

(Blocks: 1 bug, {dev-doc-complete})

unspecified
mozilla37
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(relnote-firefox 37+)

Details

(Whiteboard: [games:p1][platform-rel-Games])

Attachments

(15 attachments, 17 obsolete attachments)

9.37 KB, patch
khuey
: review+
Details | Diff | Splinter Review
16.16 KB, patch
khuey
: review+
Details | Diff | Splinter Review
30.76 KB, patch
khuey
: review+
Details | Diff | Splinter Review
4.38 KB, patch
khuey
: review+
baku
: feedback+
Details | Diff | Splinter Review
20.12 KB, patch
khuey
: review+
Details | Diff | Splinter Review
26.52 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
baku
: feedback+
Details | Diff | Splinter Review
18.60 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
baku
: feedback+
Details | Diff | Splinter Review
6.06 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
11.18 KB, patch
baku
: review+
Details | Diff | Splinter Review
17.85 KB, patch
khuey
: review+
Ben Turner (not reading bugmail, use the needinfo flag!)
: feedback+
Details | Diff | Splinter Review
23.83 KB, patch
khuey
: review+
Details | Diff | Splinter Review
22.45 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Ben Turner (not reading bugmail, use the needinfo flag!)
: feedback+
Details | Diff | Splinter Review
28.08 KB, patch
khuey
: review+
Details | Diff | Splinter Review
20.26 KB, patch
baku
: review+
Details | Diff | Splinter Review
4.64 KB, patch
khuey
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
There's a way to access an IndexedDB from a worker thread with a synchronous API.  We should support it.
Actually, per spec, workers are supposed to have access to both the sync and the async API.
I note that in the current worker setup doing this will be *excruciatingly* painful.
(Reporter)

Comment 3

6 years ago
Why so painful?  Please tell us more.
There is no XPConnect in workers, so all of the bindings must be written manually using JSAPI.
(The long term plan here is to have tools that automatically generate that goop, but those don't exist yet.)

Comment 6

6 years ago
I've started to think that we should bring back xpconnect-workers, and re-enable JS API workers
once it is easier hack them. Right now adding any features to workers is close to insanely-hard.
Please file a separate bug on that.

As for this bug. We should definitely fix it, but I think it's higher priority that we get the implementation finished and un-prefixed on the main thread first. This definitely won't be something we'll work on this quarter given the complexity involved.
Also, let's remove from tracker bug 687337. That one is about getting our main thread async API up to spec.
No longer blocks: 687337
Blocks: 553412
Component: DOM → DOM: IndexedDB
Version: Trunk → unspecified
Keywords: dev-doc-needed

Comment 9

5 years ago
Are there separate bugs for the async and sync api being available from workers?
Andrea, do we have bugs for the sync API?
I don't think so. Not yet.
https://bugzilla.mozilla.org/show_bug.cgi?id=783190
Depends on: 783190

Comment 13

5 years ago
Are there plans for implementing Async or Sync IndexedDB api in web workers in FF soon?
(In reply to Deni from comment #13)
> Are there plans for implementing Async or Sync IndexedDB api in web workers
> in FF soon?

We may try for later this calendar year but it's up in the air ATM.

Comment 15

5 years ago
I filed bug 798875 for synchronous IDB API, there's a guy who is considering to do it as his diploma thesis, but it hasn't been officially approved yet (by the university).

Comment 16

4 years ago
I'm not so much interested in the sync APIs, but having access to the async APIs from workers (in addition to the main thread) would be a killer feature for a lot of apps that are trying to shift expensive storage/processing operations off the main thread, such as games and game-like apps that need to maintain 60fps.

Also, the MDN page on IndexedDB says that the async API is available from workers even though it's currently not in FF (but is in Chrome) - it should probably be called out that it's not yet available everywhere.

Comment 17

4 years ago
+1 to Ben Vanik's comment above, the sync API has little benefit for WebGL use cases but just providing the same IDB async API in a worker context would be so amazingly awesome for making games. The other big missing piece is also to implement Transferable Objects between workers and the main context (http://updates.html5rocks.com/2011/12/Transferable-Objects-Lightning-Fast)
(In reply to max from comment #17)
> The other big missing piece
> is also to implement Transferable Objects between workers and the main
> context
> (http://updates.html5rocks.com/2011/12/Transferable-Objects-Lightning-Fast)
Not about this bug at all, but Bug 720083 was fixed for FF18.
Depends on: 845545, 853893

Updated

4 years ago
Blocks: 898706

Updated

4 years ago
Assignee: nobody → khuey

Updated

4 years ago
Depends on: 827823, 763231

Updated

4 years ago
Assignee: khuey → nobody

Updated

4 years ago
Blocks: 916204

Comment 19

4 years ago
Andrea, why did you make this bug depend on bug 783190 ?
Blocks: 916196
Alias: AsyncIDB
Depends on: 487070
Blocks: 936302

Comment 20

4 years ago
(In reply to Jan Varga [:janv] from comment #19)
> Andrea, why did you make this bug depend on bug 783190 ?
This is a mistake and should be removed: the sync API is an abstraction on-top of the of async API and should not be a blocker for this enhancement.

Andrea, if I'm wrong, would you mind correcting me?  :)
Flags: needinfo?(amarchesini)
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Created attachment 8337810 [details] [diff] [review]
patch 1 - QuotaManager helper class in workers

This is a helper class that gives access to QuotaManager mainthread only methods from workers.
Attachment #8337810 - Flags: feedback?(Jan.Varga)
Created attachment 8341165 [details] [diff] [review]
patch 2 - IDB - WIP

Still needs to have bug 914762 landed.
Created attachment 8345387 [details] [diff] [review]
patch 1 - TransactionThreadPool without DOM elements.
Attachment #8337810 - Attachment is obsolete: true
Attachment #8341165 - Attachment is obsolete: true
Attachment #8337810 - Flags: feedback?(Jan.Varga)
Attachment #8345387 - Flags: review?(bent.mozilla)
Comment on attachment 8345387 [details] [diff] [review]
patch 1 - TransactionThreadPool without DOM elements.

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

::: dom/indexedDB/IDBDatabase.cpp
@@ +791,5 @@
>  
> +void
> +IDBDatabase::RegisterTransaction(IDBTransaction* aTransaction)
> +{
> +  mTransactions.PutEntry(aTransaction);

Let's first assert that we're on the 'correct' thread, then that aTransaction is non-null, and then assert that mTransactions does not already contain aTransaction. Same below for Unregister (except reverse the Contains() assertion).

@@ +802,5 @@
> +}
> +
> +static
> +PLDHashOperator AbortTransactionsEnumerator(nsPtrHashKey<IDBTransaction>* aTransaction,
> +                                            void* aNonUsed)

Nit: Return type on its own line, and move this up into the anonymous namespace at the top of the file

@@ +815,5 @@
> +
> +void
> +IDBDatabase::AbortTransactions()
> +{
> +  mTransactions.EnumerateEntries(AbortTransactionsEnumerator, nullptr);

Assert correct thread here.

::: dom/indexedDB/IDBDatabase.h
@@ +267,5 @@
>    mozilla::dom::ContentParent* mContentParent;
>  
>    nsRefPtr<mozilla::dom::quota::Client> mQuotaClient;
>  
> +  nsTHashtable<nsPtrHashKey<IDBTransaction>> mTransactions;

I think this needs to be nsRefPtrHashKey...

::: dom/indexedDB/IDBTransaction.cpp
@@ +114,5 @@
>    transaction->mDatabaseInfo = aDatabase->Info();
>    transaction->mObjectStoreNames.AppendElements(aObjectStoreNames);
>    transaction->mObjectStoreNames.Sort();
>  
> +  aDatabase->RegisterTransaction(transaction);

You should only register once the transaction has been sent to the pool. There are a few early returns here that could leave a transaction registered incorrectly here.

@@ +180,5 @@
>    NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
>  
>    SetIsDOMBinding();
> +
> +  mId = TransactionThreadPool::GetUniqueId();

Let's set this in CreateInternal with the rest of the members?

::: dom/indexedDB/TransactionThreadPool.cpp
@@ +128,5 @@
>  
> +uint64_t
> +TransactionThreadPool::GetUniqueId()
> +{
> +  return ++gUniqueTransactionId;

This is only safe as long as we're only called on a single thread. Need some assertions here.

::: dom/indexedDB/TransactionThreadPool.h
@@ +37,5 @@
>    static TransactionThreadPool* Get();
>  
>    static void Shutdown();
>  
> +  static uint64_t GetUniqueId();

Nit: NextTransactionId?

@@ +42,5 @@
> +
> +  nsresult Dispatch(const uint64_t& aTransactionId,
> +                    const nsACString& aDatabaseId,
> +                    const nsTArray<nsString>& aObjectStoreNames,
> +                    const uint16_t aMode,

Nit: here and below, don't pass const refs for ints, or const ints for that matter.

@@ +47,5 @@
>                      nsIRunnable* aRunnable,
>                      bool aFinish,
>                      nsIRunnable* aFinishRunnable);
>  
> +  nsresult Dispatch(const uint64_t& aTransactionId,

Hm, I don't see why you need two Dispatch methods?

@@ +85,5 @@
> +
> +    uint64_t mTransactionId;
> +    const nsCString mDatabaseId;
> +    const nsTArray<nsString> mObjectStoreNames;
> +    uint16_t mMode;

Hm, does the queue really need to hold on to this information?

@@ +104,5 @@
>      {
>        MOZ_COUNT_CTOR(TransactionInfo);
>  
> +      transactionId = aTransactionId;
> +      databaseId = aDatabaseId;

These should go in an initializer list

@@ +151,5 @@
>      {
>        MOZ_COUNT_DTOR(DatabaseTransactionInfo);
>      }
>  
> +    typedef nsClassHashtable<nsUint64HashKey, TransactionInfo >

Nit: not your fault, but kill that space before >

@@ +192,5 @@
> +  TransactionQueue& GetQueueForTransaction(
> +                                    const uint64_t& aTransactionId,
> +                                    const nsACString& aDatabaseId,
> +                                    const nsTArray<nsString>& aObjectStoreNames,
> +                                    const uint16_t aMode);

These can both take a TransactionInfo* can't they?
Attachment #8345387 - Flags: review?(bent.mozilla)
> > +
> > +    uint64_t mTransactionId;
> > +    const nsCString mDatabaseId;
> > +    const nsTArray<nsString> mObjectStoreNames;
> > +    uint16_t mMode;
> 
> Hm, does the queue really need to hold on to this information?

They are used for FinishTransactionRunnable().

> @@ +192,5 @@
> > +  TransactionQueue& GetQueueForTransaction(
> > +                                    const uint64_t& aTransactionId,
> > +                                    const nsACString& aDatabaseId,
> > +                                    const nsTArray<nsString>& aObjectStoreNames,
> > +                                    const uint16_t aMode);
> 
> These can both take a TransactionInfo* can't they?

GetQueueForTransaction creates TransactionInfo if it doesn't exist yet.
Why should it receive one? For avoiding so many arguments in this method?
Created attachment 8346577 [details] [diff] [review]
patch 1 - TransactionThreadPool without DOM elements.
Attachment #8345387 - Attachment is obsolete: true
Attachment #8346577 - Flags: review?(bent.mozilla)
With the patches in bug 949682, we could migrate IDBDatabase.objectStoreNames and IDBObjectStore.indexNames to be of type "[Pure, Cached, Frozen] readonly attribute sequence<DOMString>".

That would mean we don't have to make DOMStringList work in workers. Not sure if that would simplify our lives here or not.
Comment on attachment 8346577 [details] [diff] [review]
patch 1 - TransactionThreadPool without DOM elements.

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

::: dom/indexedDB/IDBDatabase.cpp
@@ +803,5 @@
>  
> +void
> +IDBDatabase::RegisterTransaction(IDBTransaction* aTransaction)
> +{
> +  MOZ_ASSERT(NS_IsMainThread(), "Wrong thread!");

This will need to be updated to be the "correct thread" some time soon, right? It won't always be the main thread.
Attachment #8346577 - Flags: review?(bent.mozilla) → review+
Created attachment 8350102 [details] [diff] [review]
transaction.patch

rebased
Attachment #8346577 - Attachment is obsolete: true
Depends on: 956218

Updated

3 years ago
Depends on: 961049

Updated

3 years ago
Depends on: 942542

Updated

3 years ago
No longer depends on: 942542

Comment 30

3 years ago
Comment on attachment 8350102 [details] [diff] [review]
transaction.patch

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

I'm trying to remove nsIFileStorage and possibly do the same what this patch does to FileService, but I think I found at least one problem in this patch.

::: dom/indexedDB/TransactionThreadPool.cpp
@@ +466,1 @@
>    dbTransactionInfo->transactions.EnumerateRead(FindTransaction, &info);

You already found out that there are some transactions for this database id and the enumeration was there to find concrete IDBDatabase, but since you removed IDBDatabase, you don't need to enumerate.

However, I'm not sure if this is actually right. HasTransactionsForDatabase() is called by QuotaManager::HasOpenTransactions(nsPIDOMWindow*) and that gets databases for specific window (not for a database id), so the method would now return true if some other window has databases for the database id.

Comment 31

3 years ago
Heys guys, just want to let you know, that indexedDB in worker is so important for seamless webgl experience. I hope you can bring this into the amazing fox one day!
Depends on: 994190

Comment 32

3 years ago
(In reply to Markus Siebeneicher from comment #31)
> Heys guys, just want to let you know, that indexedDB in worker is so
> important for seamless webgl experience. I hope you can bring this into the
> amazing fox one day!

+100!
This has sorta become a meta bug. Development on this project is happening in the jamun project branch (http://hg.mozilla.org/projects/jamun).
Duplicate of this bug: 1032094

Updated

3 years ago
Blocks: 1032096

Comment 35

3 years ago
What's the status of this bug? Is it actively being worked on? Anything I can do to help?
Yep, we're on it, it's top priority for several of us.
Blocks: 1040611

Updated

3 years ago
No longer depends on: 961049
No longer depends on: 783190
Blocks: 1029951
No longer blocks: 1029951

Updated

3 years ago
Blocks: 877648

Updated

3 years ago
No longer blocks: 877648
Blocks: 1059784

Updated

3 years ago
Blocks: 1083089
Blocks: 1029951
I guess bent is working on this.
Assignee: amarchesini → nobody
Assignee: nobody → bent.mozilla
Depends on: 1054638
Blocks: 1102835
Created attachment 8528820 [details] [diff] [review]
Part 1: WebIDL stubs and pref, v1
Attachment #8350102 - Attachment is obsolete: true
Attachment #8528820 - Flags: review?(khuey)
Created attachment 8528822 [details] [diff] [review]
Part 2: Refactor 3rd party checks, v1
Attachment #8528822 - Flags: review?(khuey)
Attachment #8528820 - Attachment description: WebIDL stubs and pref, v1 → Part 1: WebIDL stubs and pref, v1
Created attachment 8528824 [details] [diff] [review]
Part 3: Fix open() for workers, v1
Attachment #8528824 - Flags: review?(khuey)
Created attachment 8528826 [details] [diff] [review]
Part 4: Fix wrapping of binding objects, v1
Attachment #8528826 - Flags: review?(khuey)
Created attachment 8528827 [details] [diff] [review]
Part 5: Fix error events fired at global scope, v1
Created attachment 8528828 [details] [diff] [review]
Part 6: Hook transaction lifetime to the worker event loop, v1
Attachment #8528828 - Flags: review?(khuey)
Attachment #8528827 - Flags: review?(khuey)
Created attachment 8528829 [details] [diff] [review]
Part 7: Fix blobs on workers, v1
Attachment #8528829 - Flags: review?(khuey)
Created attachment 8528830 [details] [diff] [review]
Part 8: Fix versionchange transactions on canceled workers, v1
Attachment #8528830 - Flags: review?(khuey)
Created attachment 8528831 [details] [diff] [review]
Part 9: Un-inline the RemoteInputStream (no functional changes), v1

(This is just moving code, no functional changes)
Created attachment 8528832 [details] [diff] [review]
Part 10: Add a sync message for FileReaderSync, v1
Attachment #8528832 - Flags: review?(khuey)
Created attachment 8528833 [details] [diff] [review]
Part 11: Fix the aborted transaction warning for workers, v1
Attachment #8528833 - Flags: review?(khuey)
Attachment #8528831 - Flags: review?(khuey)
Created attachment 8528835 [details] [diff] [review]
Part 12: Fix small issues revealed by tests, v1
Attachment #8528835 - Flags: review?(khuey)
Created attachment 8528836 [details] [diff] [review]
Part 13: Run properly structured mochitests as worker tests, v1
Attachment #8528836 - Flags: review?(khuey)
For those following along, I hope to land this early in the next cycle so that gaia folks can develop with indexedDB in workers for FxOS 2.2.
\o/
Comment on attachment 8528820 [details] [diff] [review]
Part 1: WebIDL stubs and pref, v1

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

Actually, Andrea, do you want to review this one?
Attachment #8528820 - Flags: review?(khuey) → review?(amarchesini)
Kyle, feel free to bounce reviews to anyone else you think could do them.
Comment on attachment 8528820 [details] [diff] [review]
Part 1: WebIDL stubs and pref, v1

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

::: dom/indexedDB/IndexedDatabaseManager.cpp
@@ +133,5 @@
>  
>  mozilla::Atomic<bool> gInitialized(false);
>  mozilla::Atomic<bool> gClosed(false);
>  mozilla::Atomic<bool> gTestingMode(false);
> +Atomic<bool> gExperimentalFeaturesEnabled(false);

mozilla::Atomic? or remove mozilla:: from the others.
Actually, drop mozilla:: from the others.

::: dom/webidl/DOMError.webidl
@@ +10,5 @@
>   * liability, trademark and document use rules apply.
>   */
>  
>  [Constructor(DOMString name, optional DOMString message = ""),
> + Exposed=(Window,Worker,System)]

Ok. This is fine but, DOMError has 3 constructors, 1 for JS and 2 for C++ only.
One of the C++ constructor uses NS_GetNameAndMessageForDOMNSResult that is main-thread only. Add an assertion there in order to prevent the use of this ctor out of main-thread.

::: dom/webidl/DOMStringList.webidl
@@ +14,1 @@
>  interface DOMStringList {

About DOMStringList, we have several subclasses and we don't want to expose all of them to workers.
In order to avoid wrong future uses, can you add an assert to the virtual method EnsureFresh of subclasses (PropertyStringList and nsDOMStyleSheetSetList) ?

::: dom/workers/WorkerScope.h
@@ +39,5 @@
>  class WorkerGlobalScope : public DOMEventTargetHelper,
>                            public nsIGlobalObject
>  {
> +  typedef mozilla::dom::indexedDB::IDBFactory IDBFactory;
> +

mozilla::dom:: should not be needed.
Attachment #8528820 - Flags: review?(amarchesini) → review+
Attachment #8528822 - Flags: review?(khuey) → review?(amarchesini)
Attachment #8528824 - Flags: review?(khuey) → review+
Attachment #8528826 - Flags: review?(khuey) → review?(amarchesini)
Attachment #8528827 - Flags: review?(khuey) → review?(amarchesini)
Attachment #8528831 - Flags: review?(khuey) → review+
Attachment #8528828 - Flags: feedback?(amarchesini)
Attachment #8528829 - Flags: feedback?(amarchesini)
Attachment #8528830 - Flags: feedback?(amarchesini)
Attachment #8528832 - Flags: feedback?(amarchesini)
Attachment #8528833 - Flags: feedback?(amarchesini)
Attachment #8528835 - Flags: feedback?(amarchesini)
Attachment #8528836 - Flags: feedback?(amarchesini)
Comment on attachment 8528822 [details] [diff] [review]
Part 2: Refactor 3rd party checks, v1

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

::: dom/base/nsGlobalWindow.cpp
@@ +259,5 @@
>  using namespace mozilla::dom;
>  using namespace mozilla::dom::ipc;
>  using mozilla::TimeStamp;
>  using mozilla::TimeDuration;
> +using mozilla::dom::indexedDB::IDBFactory;

alphabetic order?

::: dom/indexedDB/IDBFactory.cpp
@@ +143,5 @@
> +  }
> +
> +  if (NS_WARN_IF(NS_FAILED(rv))) {
> +    if (rv == NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR) {
> +      IDB_REPORT_INTERNAL_ERR();

If you want this here, then remove it in AllowedForWindowInternal.

@@ +273,5 @@
> +  nsresult rv = AllowedForWindowInternal(aWindow, getter_AddRefs(principal));
> +  if (NS_WARN_IF(NS_FAILED(rv))) {
> +    return false;
> +  }
> +

In theory rv can be NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR and you want to report the error using IDB_REPORT_INTERNAL_ERR().

@@ +293,5 @@
> +  }
> +
> +  nsIDocument* document = aWindow->GetExtantDoc();
> +  if (document->GetSandboxFlags() & SANDBOXED_ORIGIN) {
> +    return NS_ERROR_DOM_SECURITY_ERR;

Why don't we have IDB_REPORT_INTERNAL_ERR() here and in the other error conditions?

@@ +311,5 @@
> +  }
> +
> +  bool isNullPrincipal;
> +  if (NS_WARN_IF(NS_FAILED(principal->GetIsNullPrincipal(&isNullPrincipal))) ||
> +      isNullPrincipal) {

Maybe we want to NS_WARN_IF(isNullPrincipal) ?
Attachment #8528822 - Flags: review?(amarchesini) → review+
Comment on attachment 8528826 [details] [diff] [review]
Part 4: Fix wrapping of binding objects, v1

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

::: dom/indexedDB/ActorsChild.cpp
@@ +354,5 @@
>  private:
> +  template <class T>
> +  typename EnableIf<IsSame<T, IDBDatabase>::value ||
> +                      IsSame<T, IDBCursor>::value ||
> +                      IsSame<T, IDBMutableFile>::value,

indentation
Attachment #8528826 - Flags: review?(amarchesini) → review+
Comment on attachment 8528827 [details] [diff] [review]
Part 5: Fix error events fired at global scope, v1

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

r- for the innerWindowID

::: dom/indexedDB/IDBRequest.h
@@ +118,5 @@
> +  GetErrorAfterResult() const
> +#ifdef DEBUG
> +  ;
> +#else
> +  {

Do we care to AssertIsOnOwningThread() ?

::: dom/indexedDB/IndexedDatabaseManager.cpp
@@ +423,2 @@
>  
> +  nsRefPtr<DOMError> error = request->GetErrorAfterResult();

This maybe is not so important, but in theory we use 'GetFoobar()' when the return value can be null. Otherwise we use Foobar().
Can we rename GetErrorAfterResult() to 'ErrorAfterResult()' ?

Then MOZ_ASSERT(error);

@@ +490,5 @@
> +    category.AssignLiteral("chrome ");
> +  } else {
> +    category.AssignLiteral("content ");
> +  }
> +  category.AppendLiteral("javascript");

category should contain 'indexedDB' somewhere.

@@ +519,5 @@
> +                        /* aSourceLine */ EmptyString(),
> +                        init.mLineno,
> +                        /* aColumnNumber */ 0,
> +                        nsIScriptError::errorFlag,
> +                        category.get())));

In theory, you can have the window inner ID in workers too taking it from the parent window.
This will be perfect for debugging, because the webConsole will log this message correctly.

If we have take the correct window inner ID from the window when we create the factory, we can use this value.
Attachment #8528827 - Flags: review?(amarchesini) → review-
Created attachment 8533287 [details] [diff] [review]
Part 1: WebIDL stubs and pref, v1.1

(In reply to Andrea Marchesini (:baku) from comment #55)
> One of the C++ constructor uses NS_GetNameAndMessageForDOMNSResult that is
> main-thread only. Add an assertion there in order to prevent the use of this
> ctor out of main-thread.

I looked at this and it didn't actually look like NS_GetNameAndMessageForDOMNSResult is main-thread only so I didn't add the assertion... Am I missing something?

> mozilla::dom:: should not be needed.

I kinda prefer it for explicit typedefs like this though. Any objection to leaving it?
Attachment #8528820 - Attachment is obsolete: true
Attachment #8533287 - Flags: review+
Attachment #8533287 - Flags: feedback?(amarchesini)
Created attachment 8533294 [details] [diff] [review]
Part 2: Refactor 3rd party checks, v1.1

(In reply to Andrea Marchesini (:baku) from comment #56)
> alphabetic order?

It seems ok to me since we're entering another namespace... Is there something different you'd suggest?

> In theory rv can be NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR and you want to
> report the error using IDB_REPORT_INTERNAL_ERR().

No, the goal is that AllowedForWindow shouldn't log anything in the console ever. I fixed the (accidental) place where it used to do that via AllowedForWindowInternal. CreateForWindow does log stuff.

> Why don't we have IDB_REPORT_INTERNAL_ERR() here and in the other error
> conditions?

The deal with this is that the error the page sees is UnknownErr and that's incredibly not useful, so we add more info to the error console whenever we throw that one specific code. Other errors like SecurityErr are spec'd.

> Maybe we want to NS_WARN_IF(isNullPrincipal) ?

I didn't add this because I don't think it's useful.
Attachment #8533294 - Flags: review+
Attachment #8533294 - Flags: feedback?(amarchesini)
Created attachment 8533295 [details] [diff] [review]
Part 4: Fix wrapping of binding objects, v1.1
Attachment #8533295 - Flags: review+
Created attachment 8533358 [details] [diff] [review]
Part 5: Fix error events fired at global scope, v1.1

(In reply to Andrea Marchesini (:baku) from comment #58)
> Do we care to AssertIsOnOwningThread() ?

This gets asserted in the DEBUG version, but there's no point in asserting for the non-DEBUG version.

> Can we rename GetErrorAfterResult() to 'ErrorAfterResult()' ?

It still returns null sometimes.

> category should contain 'indexedDB' somewhere.

No, this is somehow special for the devtools stuff, there's a hardcoded list of stuff it understands.

> If we have take the correct window inner ID from the window when we create
> the factory, we can use this value.

Fixed.
Attachment #8528822 - Attachment is obsolete: true
Attachment #8528826 - Attachment is obsolete: true
Attachment #8528827 - Attachment is obsolete: true
Attachment #8533358 - Flags: review?(amarchesini)
Created attachment 8533407 [details] [diff] [review]
Part 11: Fix the aborted transaction warning for workers, v1.1
Attachment #8533358 - Attachment is obsolete: true
Attachment #8533358 - Flags: review?(amarchesini)
Attachment #8533407 - Flags: review?(amarchesini)
Attachment #8533287 - Flags: feedback?(amarchesini) → feedback+
Attachment #8533294 - Flags: feedback?(amarchesini) → feedback+
Comment on attachment 8528828 [details] [diff] [review]
Part 6: Hook transaction lifetime to the worker event loop, v1

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

::: dom/indexedDB/IDBRequest.cpp
@@ +544,5 @@
> +IDBOpenDBRequest::NoteComplete()
> +{
> +  AssertIsOnOwningThread();
> +  MOZ_ASSERT_IF(!NS_IsMainThread(), mWorkerFeature);
> +

write a comment about that nullifying mWorkerFeature we remove the feature in its DTOR.

::: dom/indexedDB/IDBTransaction.cpp
@@ +69,4 @@
>  } // anonymous namespace
>  
> +class IDBTransaction::WorkerFeature MOZ_FINAL
> +  : public mozilla::dom::workers::WorkerFeature

remove mozilla::dom::workers::

@@ +71,5 @@
> +class IDBTransaction::WorkerFeature MOZ_FINAL
> +  : public mozilla::dom::workers::WorkerFeature
> +{
> +  WorkerPrivate* mWorkerPrivate;
> +  IDBTransaction* mTransaction;

Maybe add a comment saying that the feature is kept alive by this IDBTransaction so we don't need to play with the refcounting.

::: dom/workers/WorkerPrivate.cpp
@@ +2081,5 @@
>  #endif
>  {
>  }
>  
> +struct WorkerPrivate::PreemptingRunnable MOZ_FINAL

NIT: I'm not a native english speaker, but this is called 'runnable', but actually it is not a runnable. What about a different name?

@@ +4390,5 @@
> +        pending->mRunnable.swap(preemptingRunnable.mRunnable);
> +        pending->mRecursionDepth = preemptingRunnable.mRecursionDepth;
> +      }
> +    }
> +

what about:

for (uint32_t index = 0; ; index < mPreemptingRunnables.Length();) {
  PreemptingRunnable& preemptingRunnable = mPreemptingRunnables[index];
  if (preemptingRunnable.mRecursionDepth == aRecursionDepth) {
    preemtingRunnable.mRunnable->Run();
    mPreemptingRunnables.RemoveAt(index);
  } else {
    ++index;
  }
}

instead allocating new memory... ?

@@ +4423,5 @@
> +  preemptingRunnable->mRecursionDepth = recursionDepth ? recursionDepth - 1 : 0;
> +
> +  // Ensure that we have a pending event so that the runnable will be guaranteed
> +  // to run.
> +  if (mPreemptingRunnables.Length() == 1 && !NS_HasPendingEvents(mThread)) {

remove mPreemtingRunnables.Length() == 1. Don't we have to add a dummy event in any case if we don't have a pending one?
Attachment #8528828 - Flags: feedback?(amarchesini) → feedback+
Comment on attachment 8528829 [details] [diff] [review]
Part 7: Fix blobs on workers, v1

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

::: dom/indexedDB/IDBDatabase.cpp
@@ +67,5 @@
>  const char kMemoryPressureObserverTopic[] = "memory-pressure";
>  const char kWindowObserverTopic[] = "inner-window-destroyed";
>  
> +class CancelableRunnableWrapper MOZ_FINAL
> +  : public nsICancelableRunnable

can you use nsCancelableRUnnable instead?

@@ +78,5 @@
> +  {
> +    MOZ_ASSERT(aRunnable);
> +  }
> +
> +  NS_DECL_THREADSAFE_ISUPPORTS

then remove this line and NS_DECL_NSIRUNNABLE && NS_DECL_NSICANCELABLERUNNABLE.

@@ +1344,5 @@
>  {
>    return IDBDatabaseBinding::Wrap(aCx, this);
>  }
>  
> +NS_IMPL_ISUPPORTS(CancelableRunnableWrapper, nsIRunnable, nsICancelableRunnable)

you can remove this line.

@@ +1348,5 @@
> +NS_IMPL_ISUPPORTS(CancelableRunnableWrapper, nsIRunnable, nsICancelableRunnable)
> +
> +NS_IMETHODIMP
> +CancelableRunnableWrapper::Run()
> +{

In theory mRunnable should exist here because Run() cannot be called after mRunnable.

So probably this code should be:

MOZ_ASSERT(mRunnable);
nsCOMPtr<nsIRunnable> runnable;
mRunnable.swap(runnable);
return runnable->Run();

correct?

::: dom/workers/RuntimeService.cpp
@@ +1686,5 @@
> +  if (!BackgroundChild::GetForCurrentThread()) {
> +    nsRefPtr<BackgroundChildCallback> callback = new BackgroundChildCallback();
> +    if (!BackgroundChild::GetOrCreateForCurrentThread(callback)) {
> +      MOZ_CRASH("Unable to connect PBackground actor for the main thread!");
> +    }

I don't think this is enough to be 100% sure that we can call
MOZ_ASSERT(BackgroundChild::GetForCurrenThread())
somewhere else. Correct?
Attachment #8528829 - Flags: feedback?(amarchesini)
No longer blocks: 1029951
Attachment #8533407 - Flags: review?(amarchesini) → review+
Attachment #8528830 - Flags: feedback?(amarchesini) → feedback+
Comment on attachment 8528833 [details] [diff] [review]
Part 11: Fix the aborted transaction warning for workers, v1

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

I suspect that this patch cannot be applied on top of patch 5.

::: dom/indexedDB/IDBDatabase.cpp
@@ +177,5 @@
>  };
>  
>  } // anonymous namespace
>  
> +class IDBDatabase::LogWarningRunnable MOZ_FINAL

wait... this is part of patch 5. Correct?
Attachment #8528833 - Flags: feedback?(amarchesini) → feedback-
Comment on attachment 8528835 [details] [diff] [review]
Part 12: Fix small issues revealed by tests, v1

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

::: dom/indexedDB/ActorsChild.cpp
@@ +2485,5 @@
> +    return NS_ERROR_UNEXPECTED;
> +  }
> +
> +  // This must always run to clean up our state.
> +  Run();

return Run(); ?
Attachment #8528835 - Flags: feedback?(amarchesini) → feedback+
Created attachment 8536664 [details] [diff] [review]
Part 11: Fix the aborted transaction warning for workers, v1.1

Rebased part 11 over changes from earlier review notes.
Attachment #8528833 - Attachment is obsolete: true
Attachment #8528833 - Flags: review?(khuey)
Attachment #8536664 - Flags: review?(khuey)
Attachment #8536664 - Flags: feedback?(amarchesini)
Comment on attachment 8528828 [details] [diff] [review]
Part 6: Hook transaction lifetime to the worker event loop, v1

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

Do we have tests that test the ordering of IDB events in the presence of sync XHR/etc?

::: dom/indexedDB/ActorsChild.cpp
@@ +1058,5 @@
>      default:
>        MOZ_CRASH("Unknown response type!");
>    }
>  
> +  GetOpenDBRequest()->NoteComplete();

Can this not return nullptr?  If not, why isn't it named OpenDBRequest()?

::: dom/workers/WorkerScope.cpp
@@ +68,5 @@
>    NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mConsole)
>    NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPerformance)
>    NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLocation)
>    NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mNavigator)
> +  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mIndexedDB)

I think this belongs in a different cset.
Attachment #8528828 - Flags: review?(khuey) → review+
Comment on attachment 8528830 [details] [diff] [review]
Part 8: Fix versionchange transactions on canceled workers, v1

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

::: dom/indexedDB/ActorsChild.cpp
@@ +1167,5 @@
>        IDBVersionChangeEvent::Create(mRequest,
>                                      type,
>                                      aCurrentVersion,
>                                      mRequestedVersion);
> +    MOZ_ASSERT(blockedEvent);

You could put the assertion outside of the if.

@@ +1370,5 @@
> +    MOZ_ASSERT(!NS_IsMainThread());
> +
> +    // Report this to the console.
> +    IDB_REPORT_INTERNAL_ERR();
> +    //MOZ_ALWAYS_TRUE(aActor->SendDeleteMe());

Why do you have commented out code in this patch?

@@ +1462,5 @@
>          IDBVersionChangeEvent::Create(mDatabase,
>                                        type,
>                                        aOldVersion,
>                                        aNewVersion.get_uint64_t());
> +      MOZ_ASSERT(versionChangeEvent);

outside the switch?
Attachment #8528830 - Flags: review?(khuey) → review-
Attachment #8528835 - Flags: review?(khuey) → review+
Created attachment 8536756 [details] [diff] [review]
Part 8: Fix versionchange transactions on canceled workers, v1.1

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #70)
> You could put the assertion outside of the if.

But then you can't tell which case failed from the stdout. I like having the different line numbers in the output.

> Why do you have commented out code in this patch?

Fixed.

> outside the switch?

Again with being able to tell which case was hit via stdout.
Attachment #8528830 - Attachment is obsolete: true
Attachment #8536756 - Flags: review?(khuey)
Attachment #8536756 - Flags: feedback+
Created attachment 8536776 [details] [diff] [review]
Part 2.5: Add factory methods for workers

This patch was somehow missing from this bug... Oops.
Attachment #8536776 - Flags: review?(khuey)
Created attachment 8536784 [details] [diff] [review]
Part 6: Hook transaction lifetime to the worker event loop, v1.1

(In reply to Andrea Marchesini (:baku) from comment #64)
> remove mozilla::dom::workers::

Unfortunately I can't because the new class name is also WorkerFeature so I have to distinguish it from the base.

> NIT: I'm not a native english speaker, but this is called 'runnable', but
> actually it is not a runnable. What about a different name?

PreemptingRunnableInfo!

> for (uint32_t index = 0; ; index < mPreemptingRunnables.Length();) {
>   PreemptingRunnable& preemptingRunnable = mPreemptingRunnables[index];
>   if (preemptingRunnable.mRecursionDepth == aRecursionDepth) {
>     preemtingRunnable.mRunnable->Run();
>     mPreemptingRunnables.RemoveAt(index);
>   } else {
>     ++index;
>   }
> }

Unfortunately I have to guard against the runnable mutating the array, so I can't do this.

> remove mPreemtingRunnables.Length() == 1. Don't we have to add a dummy event
> in any case if we don't have a pending one?

No, we are only worried about the specific case tested for here. If the length is > 1 then we must have already dispatched a dummy runnable (assuming that there were no other events in the queue at the time).

> Do we have tests that test the ordering of IDB events in the presence of
> sync XHR/etc?

Not specifically, no, but all IPDL messages are dispatched to the main event queue only so there's no possibility that they could run during one of our sync loops.

> Can this not return nullptr?  If not, why isn't it named OpenDBRequest()?

It can return null after ActorDestroy. I added another assertion here just to make it more clear.

> I think this belongs in a different cset.

It's now in Part 2.5!
Attachment #8536784 - Flags: review+
Attachment #8536784 - Flags: feedback+
Attachment #8528828 - Attachment is obsolete: true
Created attachment 8536793 [details] [diff] [review]
Part 7: Fix blobs on workers, v1.1

(In reply to Andrea Marchesini (:baku) from comment #65)
> can you use nsCancelableRUnnable instead?

Yep

> then remove this line and NS_DECL_NSIRUNNABLE &&
> NS_DECL_NSICANCELABLERUNNABLE.

Well, I want my own name in the leak stats so I left NS_DECL_ISUPPORTS_INHERITED. Then I have to keep the other two lines because nsCancelableRunnable actually implements Run/Cancel with no-ops (I have no idea why that makes any sense).

> you can remove this line.

Changed to NS_IMPL_ISUPPORTS_INHERITED0

> In theory mRunnable should exist here because Run() cannot be called after
> mRunnable.

Well, that's what the worker code does currently... But since nsIRunnable/nsICancelable are so generic I think it's smarter to have code here to handle misuse.

> I don't think this is enough to be 100% sure that we can call
> MOZ_ASSERT(BackgroundChild::GetForCurrenThread())
> somewhere else. Correct?

It's not bulletproof, no, but I don't think it's worth worrying about at the moment. In order for us to lose this race we'd have to have PBackground thread startup be somehow slower than Necko loading the worker's script.
Attachment #8528829 - Attachment is obsolete: true
Attachment #8528829 - Flags: review?(khuey)
Attachment #8536793 - Flags: review?(khuey)
Attachment #8536793 - Flags: feedback?(amarchesini)
(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #74)
> In order for us to lose this race we'd have to have PBackground
> thread startup be somehow slower than Necko loading the worker's script.

Fixing that isn't very difficult (could just artificially delay the scriptloader while waiting for PBackground to connect), but I don't think it's worth the effort really.
Created attachment 8536801 [details] [diff] [review]
Part 7: Fix blobs on workers, v1.1

Actually, I don't want the threadsafe refcount, so I just left CancelableRunnableWrapper pretty much as it was.
Attachment #8536793 - Attachment is obsolete: true
Attachment #8536793 - Flags: review?(khuey)
Attachment #8536793 - Flags: feedback?(amarchesini)
Attachment #8536801 - Flags: review?(khuey)
Attachment #8536801 - Flags: feedback?(amarchesini)
Comment on attachment 8533407 [details] [diff] [review]
Part 11: Fix the aborted transaction warning for workers, v1.1

Oops, this was actually part 11.
Attachment #8533407 - Attachment description: Part 5: Fix error events fired at global scope, v1.2 → Part 11: Fix the aborted transaction warning for workers, v1.1
Attachment #8536664 - Attachment is obsolete: true
Attachment #8536664 - Flags: review?(khuey)
Attachment #8536664 - Flags: feedback?(amarchesini)
Created attachment 8536819 [details] [diff] [review]
Part 5: Fix error events fired at global scope, v1
Attachment #8536819 - Flags: review?(amarchesini)
Attachment #8536776 - Flags: review?(khuey) → review?(amarchesini)
(In reply to Andrea Marchesini (:baku) from comment #67)
> return Run(); ?

No, nsICancelable specifies what should be returned, and when.
Comment on attachment 8536801 [details] [diff] [review]
Part 7: Fix blobs on workers, v1.1

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

::: dom/indexedDB/IDBObjectStore.cpp
@@ +612,5 @@
> +    if (NS_IsMainThread()) {
> +      if (aDatabase && aDatabase->GetParentObject()) {
> +        parent = aDatabase->GetParentObject();
> +      } else {
> +        parent  = xpc::NativeGlobal(JS::CurrentGlobalOrNull(aCx));

nit, extra whitespace after parent
Attachment #8536801 - Flags: review?(khuey)
Attachment #8536801 - Flags: review+
Attachment #8536801 - Flags: feedback?(amarchesini)
Comment on attachment 8536819 [details] [diff] [review]
Part 5: Fix error events fired at global scope, v1

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

::: dom/indexedDB/IDBFactory.cpp
@@ +171,5 @@
>    factory->mTabChild = TabChild::GetFrom(aWindow);
>    factory->mInnerWindowID = aWindow->WindowID();
>    factory->mPrivateBrowsingMode =
>      loadContext && loadContext->UsePrivateBrowsing();
> +  factory->mUseInnerWindowID = true;

This should not be needed.

@@ +317,5 @@
>    factory->mOwningObject = aOwningObject;
>    mozilla::HoldJSObjects(factory.get());
>  
> +  if (aOptionalInnerWindowID.WasPassed()) {
> +    factory->mInnerWindowID = aOptionalInnerWindowID.Value();

This could be written like:

 factory->mInnerWindowID = aOptionalInnerWindowID.Value();
 MOZ_ASSERT(factory->mInnerWindowID);

@@ +438,5 @@
> +uint64_t
> +IDBFactory::InnerWindowID() const
> +{
> +  AssertIsOnOwningThread();
> +  MOZ_ASSERT(mUseInnerWindowID);

you don't need to use mUseInnerWindowID because mInnerWindowID is always > 0.

::: dom/indexedDB/IndexedDatabaseManager.cpp
@@ +36,3 @@
>  #include "nsThreadUtils.h"
>  #include "prlog.h"
> +#include "WorkerScope.h"

alphabetic order.
Attachment #8536819 - Flags: review?(amarchesini) → review+
Blocks: 940273
Comment on attachment 8528832 [details] [diff] [review]
Part 10: Add a sync message for FileReaderSync, v1

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

::: dom/ipc/Blob.cpp
@@ +239,2 @@
>  ConstructFileDescriptorSet(ManagerType* aManager,
> +                           nsTArray<FileDescriptor>& aFDs,

Try a move reference here.  It's more semantically correct.

@@ +276,5 @@
> +}
> +
> +void
> +OptionalFileDescriptorSetToFDs(OptionalFileDescriptorSet& aOptionalSet,
> +                               nsTArray<FileDescriptor>& aFDs)

You could return an nsTArray&& too.
Attachment #8528832 - Flags: review?(khuey) → review+
Attachment #8528832 - Flags: feedback?(amarchesini)
Attachment #8536756 - Flags: review?(khuey) → review+
Comment on attachment 8536776 [details] [diff] [review]
Part 2.5: Add factory methods for workers

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

::: dom/workers/WorkerPrivate.cpp
@@ +91,5 @@
>  #include "WorkerScope.h"
>  
> +#ifdef XP_WIN
> +#undef PostMessage
> +#endif

Nooooooooooooooooooooooooooooooooooooo

::: dom/workers/WorkerScope.cpp
@@ +321,5 @@
>  already_AddRefed<IDBFactory>
>  WorkerGlobalScope::GetIndexedDB(ErrorResult& aErrorResult)
>  {
> +  mWorkerPrivate->AssertIsOnWorkerThread();
> +  MOZ_ASSERT(mWorkerPrivate->IsIndexedDBAllowed());

We never check this before asserting it.  This needs to return null (here or somewhere else).
Attachment #8536776 - Flags: review?(amarchesini) → review+
Comment on attachment 8528836 [details] [diff] [review]
Part 13: Run properly structured mochitests as worker tests, v1

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

::: dom/indexedDB/test/helpers.js
@@ +60,5 @@
> +    let src = scripts[i].src;
> +    let match = src.match(/indexedDB\/test\/unit\/(test_[^\/]+\.js)$/);
> +    if (match && match.length == 2) {
> +      testScriptPath = src;
> +      testScriptFilename = match[1];

break?

::: dom/indexedDB/test/unit/test_setVersion_events.js
@@ +2,5 @@
>   * Any copyright is dedicated to the Public Domain.
>   * http://creativecommons.org/publicdomain/zero/1.0/
>   */
>  
> +let disableWorkerTest = "This test uses SpecialPowers";

This test probably can and should be fixed.
Attachment #8528836 - Flags: review?(khuey)
Attachment #8528836 - Flags: review+
Attachment #8528836 - Flags: feedback?(amarchesini)
Created attachment 8537433 [details] [diff] [review]
Part 14: Loose ends
Attachment #8537433 - Flags: review?(khuey)
Created attachment 8537434 [details] [diff] [review]
Part 14: Loose ends
Attachment #8537433 - Attachment is obsolete: true
Attachment #8537433 - Flags: review?(khuey)
Attachment #8537434 - Flags: review?(khuey)
Attachment #8537434 - Flags: review?(khuey) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d0ed89e7c58
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d40b95ffdbf
Depends on: 1112716
https://hg.mozilla.org/mozilla-central/rev/9d0ed89e7c58
https://hg.mozilla.org/mozilla-central/rev/8d40b95ffdbf
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
We should blog about this on hacks once it has moved down a train or two.
Release Note Request (optional, but appreciated)
[Why is this notable]: Needed by people working heavily with IndexedDB
[Suggested wording]: Worker threads have access to IndexedDB
[Links (documentation, blog post, etc)]: See comment 90

Sebastian
relnote-firefox: --- → ?
Comment hidden (me-too)
Blocks: 1113340
relnoted as "IndexDB now accessible from worker threads"
relnote-firefox: ? → 37+
Please make sure that you spell the name of the feature correctly ;)
Flags: needinfo?(lmandel)
Oh. That's a great typo. Thanks for catching it!
Flags: needinfo?(lmandel)
Covered in https://developer.mozilla.org/en-US/docs/Web/API/Worker/Functions_and_classes_available_to_workers
Keywords: dev-doc-needed → dev-doc-complete
No longer blocks: 1102835

Updated

2 years ago
Depends on: 1152174
Blocks: 1161063
Whiteboard: [games:p1]
Whiteboard: [games:p1] → [games:p1][platform-rel-Games]
You need to log in before you can comment on or make changes to this bug.