Closed Bug 976311 Opened 6 years ago Closed 6 years ago

DataStore.onchange is called when not needed.

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox28 --- wontfix
firefox29 --- wontfix
firefox30 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: baku, Assigned: baku)

References

Details

Attachments

(1 file)

No description provided.
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}"}
Attached patch event.patchSplinter Review
Attachment #8381324 - Flags: review?(ehsan)
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.
Attachment #8381324 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/0c8703fab0c9
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
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.
(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.
(And the fact that our test harness doesn't go orange is _another_ bug worth filing!)
Blocks: 979199
Comment on attachment 8381324 [details] [diff] [review]
event.patch

Blocks bug 979199 and bug 985042 from landing.
Attachment #8381324 - Flags: approval-mozilla-b2g28?
Attachment #8381324 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.