1006562 - WorkerDataStoreCursor.store should be equal to the WorkerDataStore which owns the cursor

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Gene Lian (I already quit Mozilla)]

This is a follow-up for bug 949325. We need to fix the test of:

  is(cursor.store, gStore, "Cursor.store is the store");

in the file_sync_worker.js.

The tricky part is cursor.store will return a newly created WorkerDataStore shell which proxie to the real DataStore. However, we shouldn't create a new WorkerDataStore to return. Instead, we should return the WorkerDataStore which owns the WorkerDataStoreCursor.

I already had a patch for this. Will upload it tomorrow.

Comment 1

[Gene Lian (I already quit Mozilla)]

Forgot to clean up the CC list when cloning. Sorry for the noise.

Comment 2

[Gene Lian (I already quit Mozilla)]

Attachment: Part 1, minor code clean-up

Comment 3

[Gene Lian (I already quit Mozilla)]

Attachment: Part 2, WorkerDataStoreCursor.store == WorkerDataStore

Comment 4

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8418492 [details] [diff] [review]
Part 1, minor code clean-up

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

::: dom/workers/DataStore.cpp
@@ +360,5 @@
>    }
>  };
>  
> +// A DataStoreRunnable to run DataStore::Clear(...) on the main thread.
> +class DataStoreClearRunnable MOZ_FINAL : public DataStoreRunnable

I just move DataStoreClearRunnable to a better place which collects all the runnables, so that the real API implementations can be listed consecutively and to be viewed easily.

Comment 5

[Andrea Marchesini [:baku]]

Comment on attachment 8418492 [details] [diff] [review]
Part 1, minor code clean-up

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

::: dom/workers/DataStore.cpp
@@ +36,5 @@
>  {
>    JSContext* cx = aGlobal.GetContext();
>    WorkerPrivate* workerPrivate = GetWorkerPrivateFromContext(cx);
>    MOZ_ASSERT(workerPrivate);
>    workerPrivate->AssertIsOnWorkerThread();

Why all of this if you want to call NS_NOTREACHED immediately after?

Comment 6

[Gene Lian (I already quit Mozilla)]

(In reply to Andrea Marchesini (:baku) from comment #5)
> Comment on attachment 8418492 [details] [diff] [review]
> Part 1, minor code clean-up
> 
> Review of attachment 8418492 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/workers/DataStore.cpp
> @@ +36,5 @@
> >  {
> >    JSContext* cx = aGlobal.GetContext();
> >    WorkerPrivate* workerPrivate = GetWorkerPrivateFromContext(cx);
> >    MOZ_ASSERT(workerPrivate);
> >    workerPrivate->AssertIsOnWorkerThread();
> 
> Why all of this if you want to call NS_NOTREACHED immediately after?

Yeap, we can remove these.

Comment 7

[Gene Lian (I already quit Mozilla)]

Attachment: Part 1, minor code clean-up, V1.1

Comment 8

[Gene Lian (I already quit Mozilla)]

Attachment: Part 1, minor code clean-up, V1.1

Sorry, wrong patch. This one is correct.

Comment 9

[Gene Lian (I already quit Mozilla)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=98fff4e6c5c0

Comment 10

[Gene Lian (I already quit Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/65ba102d73b2
https://hg.mozilla.org/integration/mozilla-inbound/rev/729515d1cfff

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/65ba102d73b2
https://hg.mozilla.org/mozilla-central/rev/729515d1cfff