The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla25

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: philikon, Assigned: jessica)

Tracking

unspecified
mozilla25
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][lang=js][mentor=vicamo][fixed-in-birch], URL)

Attachments

(1 attachment, 3 obsolete attachments)

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

Updated

5 years ago
> * {handle|send}ChromeMessage
> * {handle|send}MainthreadMessage
> * ...

Can you please which name should be preferred here.
(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?
Created attachment 633832 [details] [diff] [review]
Patch(v1)
Attachment #633832 - Flags: review?(philipp)
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.)
Attachment #633832 - Flags: review?(philipp) → feedback+

Updated

5 years ago
Assignee: nobody → jigneshhk1992

Comment 5

4 years ago
Philikin's no longer involved with b2g. Who can provide some forward direction here?
Whiteboard: [good first bug][lang=js][mentor=philikon] → [good first bug][lang=js]
Keeping it open for all. If some one would like to work on it :)
Assignee: jigneshhk1992 → nobody
Whiteboard: [good first bug][lang=js] → [good first bug][lang=js][mentor=vicamo]
(Assignee)

Updated

4 years ago
Assignee: nobody → jjong
(Assignee)

Comment 7

4 years ago
Created attachment 778347 [details] [diff] [review]
rename {handle|send}DOMMessage in ril_worker.js patch
Attachment #633832 - Attachment is obsolete: true
Attachment #778347 - Flags: review?(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.
Attachment #778347 - Flags: review?(vyang) → review+
(Assignee)

Comment 9

4 years ago
Created attachment 778360 [details] [diff] [review]
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
Attachment #778347 - Attachment is obsolete: true
Attachment #778360 - Flags: review+
(Assignee)

Comment 10

4 years ago
Created attachment 779646 [details] [diff] [review]
rename {handle|send}DOMMessage in ril_worker.js patch

rebased.

try results:
https://tbpl.mozilla.org/?tree=Try&rev=4f200a1e3a3c
Attachment #778360 - Attachment is obsolete: true
https://hg.mozilla.org/projects/birch/rev/7b9a998bde77
Whiteboard: [good first bug][lang=js][mentor=vicamo] → [good first bug][lang=js][mentor=vicamo][fixed-in-birch]
https://hg.mozilla.org/mozilla-central/rev/7b9a998bde77
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25

Updated

4 years ago
Blocks: 899484
You need to log in before you can comment on or make changes to this bug.