[Datastore] System message is not received when the datastore has changed

RESOLVED FIXED in mozilla36

Status

()

defect
P1
major
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jmcf, Assigned: baku)

Tracking

Trunk
mozilla36
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

Reporter

Description

5 years ago
See attached example app.
Reporter

Updated

5 years ago
Summary: [Datastore] Don't get system message when the datastore has changed → [Datastore] System message is not received when the datastore has changed
Assignee

Comment 1

5 years ago
I don't know the details of 'messages' in the manifest, but I see that the registration for datastore-update-contacts happens just in datastore.js. This is loaded by ds.html.

"messages" in the manifest file is valid only for broadcast messages. datastore-update-contacts is not a broadcast message because we send it only to the correct apps (the apps that have access to that particular datastore).

So what you probably want to do is to call navigator.mozSetMessageHandler() in index.js.
Flags: needinfo?(jmcf)
Reporter

Comment 2

5 years ago
(In reply to Andrea Marchesini (:baku) from comment #1)
> I don't know the details of 'messages' in the manifest, but I see that the
> registration for datastore-update-contacts happens just in datastore.js.
> This is loaded by ds.html.
> 
> "messages" in the manifest file is valid only for broadcast messages.

That's not true. messages is to declare the messages the app will be dealing with and those are not broadcast messages, they are messages to be received by the app, for instance messages coming from the alarms API. 

> datastore-update-contacts is not a broadcast message because we send it only
> to the correct apps (the apps that have access to that particular datastore).

Yes, but as it happens with any other API the sys msg should be declared in the manifest 

> 
> So what you probably want to do is to call navigator.mozSetMessageHandler()
> in index.js.

yes, I have done that and the message does not arrive.
Flags: needinfo?(jmcf)
Reporter

Updated

5 years ago
blocking-b2g: --- → 2.2?
Reporter

Comment 3

5 years ago
ni Ehsan to confirm what was said in comment #2
Flags: needinfo?(ehsan.akhgari)
Reporter

Updated

5 years ago
Blocks: 989932

Comment 4

5 years ago
(In reply to Jose Manuel Cantera from comment #2)
> (In reply to Andrea Marchesini (:baku) from comment #1)
> > I don't know the details of 'messages' in the manifest, but I see that the
> > registration for datastore-update-contacts happens just in datastore.js.
> > This is loaded by ds.html.
> > 
> > "messages" in the manifest file is valid only for broadcast messages.
> 
> That's not true. messages is to declare the messages the app will be dealing
> with and those are not broadcast messages, they are messages to be received
> by the app, for instance messages coming from the alarms API. 

I think that's how broadcast messages work.  But the datastore code doesn't broadcast a message, it sends it to a specific app.

> > datastore-update-contacts is not a broadcast message because we send it only
> > to the correct apps (the apps that have access to that particular datastore).
> 
> Yes, but as it happens with any other API the sys msg should be declared in
> the manifest 

Note that broadcast and targeted messages are different.

> > So what you probably want to do is to call navigator.mozSetMessageHandler()
> > in index.js.
> 
> yes, I have done that and the message does not arrive.

I'll defer to Andrea for the rest.
Flags: needinfo?(ehsan.akhgari)
Hi folks, I added a new attachment contactslistener.zip with an app that manages to get notified of updates, but working kind of the way that Jose comments.

In the |messages| section we listen to |update.html| (not |index.html|), also requested the permission |datastore-update-contacts|, and access to the 'contacts' datastores.

When I modify any app that defines a contact datastore, I can see how (after some secons, thanks for the throttling implementation) the document defined in |messages| it's opened.

In that |update.html| we load a |update.js| that has no problem in being loaded, but there we setup the |mozSetMessageHandler| and we never handle that message.

Reading Baku's and Eshan's comments, sounds to me that being a direct message (not broadcasted), we don't have to filter it with mozSetMessageHandler, but my question is, how do we (or can we) extract information from that direct message?

Thanks!
Posted file contactslistener.zip (obsolete) —
Attachment #8506940 - Attachment is obsolete: true
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #6)

> 
> Reading Baku's and Eshan's comments, sounds to me that being a direct
> message (not broadcasted), we don't have to filter it with
> mozSetMessageHandler, but my question is, how do we (or can we) extract
> information from that direct message?

Looking at the notification system tests in:

http://mxr.mozilla.org/mozilla-central/source/dom/datastore/tests/file_notify_system_message.html

We should get the message when doing:

navigator.mozSetMessageHandler('datastore-update-contacts', function(evt) {
  console.log('We have contacts updated!');
});


> 
> Thanks!
Some good news, just tried one of the apps that adds contacts into a 'contacts' Datastore, (a linkedin example in dev_apps), importing around 400 contacts just triggered the update message once, so the throttling mechanism seems that is working nice.

Comment 10

5 years ago
Andrea, looks like the issue is that by the time that the app calls mozSetMessageHandler after being woken up, the message is gone.  Can you please help?  Thanks!
Flags: needinfo?(amarchesini)
Assignee

Updated

5 years ago
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Assignee

Comment 11

5 years ago
Posted patch ds.patchSplinter Review
Attachment #8507049 - Flags: review?(fabrice)
Assignee

Comment 12

5 years ago
Comment on attachment 8507049 [details] [diff] [review]
ds.patch

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

::: dom/messages/SystemMessagePermissionsChecker.jsm
@@ +223,5 @@
>      debug("isSystemMessagePermittedToRegister(): " +
>            "aSysMsgName: " + aSysMsgName + ", " +
>            "aManifestURL: " + aManifestURL + ", " +
>            "aManifest: " + JSON.stringify(aManifest));
>  

I don't need to move this block. In the next version of this patch I do: 

 if (this.isDataStoreSystemMessage(aSysMsgName) &&
    this.canDeliverDataStoreSystemMessage(aSysMsgName, aManifestURL)) {
    return true;
 }

just here.
Attachment #8507049 - Flags: review?(fabrice) → review+
https://hg.mozilla.org/mozilla-central/rev/0660519a81b7
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Unable to verify this bug because it seems like Back-end issue
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-] → [QAnalyst-Triage-][QAnalyst-verify-]
Flags: needinfo?(ktucker)
triage: unclear about the user impact and STR.
removing the nom for now, but please feel free to renominate with clearer information.
blocking-b2g: 2.2? → ---
Flags: needinfo?(jmcf)
Reporter

Comment 18

4 years ago
(In reply to Wesley Huang [:wesley_huang] (EPM) (NI me) from comment #17)
> triage: unclear about the user impact and STR.
> removing the nom for now, but please feel free to renominate with clearer
> information.

no problem Wesley. this bug has to do with the Contacts Data Refactoring and it is unclear to me when that work would be resumed.

thanks!
Flags: needinfo?(jmcf)
You need to log in before you can comment on or make changes to this bug.