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

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect, P1)

Description

[Jose Manuel Cantera]

Attachment: contacts-manager.zip

See attached example app.

Comment 1

[Andrea Marchesini [:baku]]

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.

Comment 2

[Jose Manuel Cantera]

(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.

Comment 3

[Jose Manuel Cantera]

ni Ehsan to confirm what was said in comment #2

Comment 4

[(no longer active)]

(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.

Comment 5

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

Attachment: contactslistener.zip

Comment 6

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

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!

Comment 7

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

Attachment: contactslistener.zip

Comment 8

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

(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!

Comment 9

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

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

[(no longer active)]

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!

Comment 11

[Andrea Marchesini [:baku]]

Attachment: ds.patch

Comment 12

[Andrea Marchesini [:baku]]

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.

Comment 13

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0660519a81b7

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/0660519a81b7

Comment 15

[Fernando Jiménez Moreno [:ferjm]]

Attachment: contactslistener.zip

Test app updated according to https://bugzilla.mozilla.org/show_bug.cgi?id=1091748#c2

Comment 16

[pete siphantong [:Petes]]

Unable to verify this bug because it seems like Back-end issue

Comment 17

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: unclear about the user impact and STR.
removing the nom for now, but please feel free to renominate with clearer information.

Comment 18

[Jose Manuel Cantera]

(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!