976311 - DataStore.onchange is called when not needed.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Comment 1

[Francisco Jordano [:arcturus] [:francisco]]

With 2 datastores 'contacts' and this code for adding listeners:


https://pastebin.mozilla.org/4382473

When we modify one DS, we get the onchange fired twice, here is the output from the previous code:
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:31 in onStoreChange: ------------------ DS CHANGE ----------------
E/GeckoConsole( 6778): Content JS LOG at debugger eval code:1 in anonymous: Changed!
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:34 in onStoreChange: owner: app://provider1.gaiamobile.org/manifest.webapp
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:36 in onStoreChange: revision: {1b4a320f-792a-44b8-a1a0-afd327abdac1}
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:37 in onStoreChange: id: 114
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:38 in onStoreChange: operation: updated
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:43 in onStoreChange: {"app://provider1.gaiamobile.org/manifest.webapp":"{1b4a320f-792a-44b8-a1a0-afd327abdac1}","app://provider2.gaiamobile.org/manifest.webapp":"{7318445e-9c9c-4259-9cbb-6e7f2a3ba55f}"}
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:31 in onStoreChange: ------------------ DS CHANGE ----------------
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:34 in onStoreChange: owner: app://provider2.gaiamobile.org/manifest.webapp
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:36 in onStoreChange: revision: {1b4a320f-792a-44b8-a1a0-afd327abdac1}
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:37 in onStoreChange: id: 114
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:38 in onStoreChange: operation: updated
E/GeckoConsole( 6650): Content JS LOG at app://system.gaiamobile.org/js/contacts.js:43 in onStoreChange: {"app://provider1.gaiamobile.org/manifest.webapp":"{1b4a320f-792a-44b8-a1a0-afd327abdac1}","app://provider2.gaiamobile.org/manifest.webapp":"{1b4a320f-792a-44b8-a1a0-afd327abdac1}"}

Comment 2

[Andrea Marchesini [:baku]]

Attachment: event.patch

Comment 3

[(no longer active)]

Comment on attachment 8381324 [details] [diff] [review]
event.patch

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

Can you please push this to the try server enabling the data store tests to make sure that the test actually passes on the emulator?

::: dom/datastore/DataStore.jsm
@@ +309,5 @@
>        debug("Wrong message: " + aMessage.name);
>        return;
>      }
>  
> +    if (aMessage.data.owner != this._owner ||

Can you please add a comment stating what this condition means.

Comment 4

[Andrea Marchesini [:baku]]

https://tbpl.mozilla.org/?tree=Try&rev=9e29601331fa

Comment 5

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c8703fab0c9

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0c8703fab0c9

Comment 7

[Gene Lian (I already quit Mozilla)]

I just tried to run:

./mach mochitest-plain dom/datastore/tests/test_basic.html

All green but eventually it will show:

0:14.47 TEST-INFO | unknown test url | All done
0:14.61 System JS : ERROR resource://gre/modules/DataStore.jsm:328 - TypeError: can't access dead object
0:21.93 INFO | runtests.py | Application ran for: 0:00:17.649658
0:21.93 INFO | zombiecheck | Reading PID log: /tmp/tmpZiXyRGpidlog
0:21.96 WARNING | leakcheck | refcount logging is off, so leaks can't be detected!
0:21.96 runtests.py | Running tests: end.

where the DataStore.jsm:328 is the following line changed in this patch.

let event = new self._window.DataStoreChangeEvent('change',
                                                  aMessage.data.message);

I'm not sure if it's harmful since tests are all green.

Comment 8

[(no longer active)]

(In reply to comment #7)
> I just tried to run:
> 
> ./mach mochitest-plain dom/datastore/tests/test_basic.html
> 
> All green but eventually it will show:
> 
> 0:14.47 TEST-INFO | unknown test url | All done
> 0:14.61 System JS : ERROR resource://gre/modules/DataStore.jsm:328 - TypeError:
> can't access dead object
> 0:21.93 INFO | runtests.py | Application ran for: 0:00:17.649658
> 0:21.93 INFO | zombiecheck | Reading PID log: /tmp/tmpZiXyRGpidlog
> 0:21.96 WARNING | leakcheck | refcount logging is off, so leaks can't be
> detected!
> 0:21.96 runtests.py | Running tests: end.
> 
> where the DataStore.jsm:328 is the following line changed in this patch.
> 
> let event = new self._window.DataStoreChangeEvent('change',
>                                                   aMessage.data.message);
> 
> I'm not sure if it's harmful since tests are all green.

It is definitely a bug.  Please file it.

Comment 9

[(no longer active)]

(And the fact that our test harness doesn't go orange is _another_ bug worth filing!)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8381324 [details] [diff] [review]
event.patch

Blocks bug 979199 and bug 985042 from landing.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/b0001efe728d