761732 - B2G RIL: rename {handle|send}DOMMessage in ril_worker.js

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Philipp von Weitershausen [:philikon]]

We don't really notify the DOM directly from ril_worker.js. With e10s we don't even notify the *same process* that the DOM lives in. Suggested names:

* {handle|send}ChromeMessage
* {handle|send}MainthreadMessage
* ...

Comment 1

[Jignesh Kakadiya [:jhk]]

> * {handle|send}ChromeMessage
> * {handle|send}MainthreadMessage
> * ...

Can you please which name should be preferred here.

Comment 2

[Philipp von Weitershausen [:philikon]]

(In reply to Jignesh Kakadiya [:jhk] from comment #1)
> > * {handle|send}ChromeMessage
> > * {handle|send}MainthreadMessage
> > * ...
> 
> Can you please which name should be preferred here.

Sure, I'll toss a coin. ;) sendChromeMessage is shorter, so let's go with that?

Comment 3

[Jignesh Kakadiya [:jhk]]

Attachment: Patch(v1)

Comment 4

[Philipp von Weitershausen [:philikon]]

Comment on attachment 633832 [details] [diff] [review]
Patch(v1)

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

Great, thanks! We need to find a good moment to land this without breaking any of the other patches that are about to land. Would you mind keeping this patch up to date until we can find such a moment (note: unfortunately since this isn't a blocker, it may very well be that this moment will not happen very soon.)

Comment 5

[Josh Matthews [:jdm]]

Philikin's no longer involved with b2g. Who can provide some forward direction here?

Comment 6

[Jignesh Kakadiya [:jhk]]

Keeping it open for all. If some one would like to work on it :)

Comment 7

[Jessica Jong [:jessica] (inactive)]

Attachment: rename {handle|send}DOMMessage in ril_worker.js patch

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 778347 [details] [diff] [review]
rename {handle|send}DOMMessage in ril_worker.js patch

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

Nice patch! r=me with nits addressed. :)

::: dom/system/gonk/ril_worker.js
@@ +2446,5 @@
>          if (this.IMEI == null) {
>            this.getIMEI({mmi: true});
>            return;
>          }
> +        // If we already had the device's IMEI, we just send it to chrome

nit: period at line end.

@@ +4452,5 @@
>     *
>     * @param message
>     *        Object containing the message. Messages are supposed
>     */
> +  handleChromeMessage: function handleMessage(message) {

nit: s/handleMessage/handleChromeMessage/ as well.

Comment 9

[Jessica Jong [:jessica] (inactive)]

Attachment: rename {handle|send}DOMMessage in ril_worker.js

Two changes made to address comments in Comment 8:

> +        // If we already had the device's IMEI, we just send it to chrome
added period at end of line

> +  handleChromeMessage: function handleMessage(message) {
replaced handleMessage with handleChromeMessage

Comment 10

[Jessica Jong [:jessica] (inactive)]

Attachment: rename {handle|send}DOMMessage in ril_worker.js patch

rebased.

try results:
https://tbpl.mozilla.org/?tree=Try&rev=4f200a1e3a3c

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/projects/birch/rev/7b9a998bde77

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7b9a998bde77