Last Comment Bug 779621 - B2G RIL: Give message.type a better name to avoid conflicts
: B2G RIL: Give message.type a better name to avoid conflicts
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Other Branch
: ARM Gonk (Firefox OS)
-- normal (vote)
: mozilla17
Assigned To: Philipp von Weitershausen [:philikon]
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2012-08-01 13:51 PDT by Philipp von Weitershausen [:philikon]
Modified: 2012-08-07 07:36 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

v1 (36.79 KB, patch)
2012-08-01 13:51 PDT, Philipp von Weitershausen [:philikon]
marshall: review+
Details | Diff | Splinter Review

Description User image Philipp von Weitershausen [:philikon] 2012-08-01 13:51:03 PDT
When we send messages from ril_worker to RadioInterfaceLayer and back, we use the `.type` attribute in the JSON payload to decide what kind of message it is. This can easily conflict with code that actually wants to use the `type` attribute itself. I think we already have one potential conflict in some SIM I/O code, others are just waiting to happen (I kind of walked into that trap just now doing some refactoring.)
Comment 1 User image Philipp von Weitershausen [:philikon] 2012-08-01 13:51:46 PDT
Created attachment 648066 [details] [diff] [review]
Comment 2 User image Marshall Culpepper [:marshall_law] 2012-08-02 08:51:09 PDT
Comment on attachment 648066 [details] [diff] [review]

Review of attachment 648066 [details] [diff] [review]:

Looks good! r+ with either removing the change to pdptype, or updating the other references (see comments below)

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1601,5 @@
>                               apn: apn,
>                               user: user,
>                               passwd: passwd,
>                               chappap: chappap,
> +                             pdprilMessageType: pdptype});

Hrm, it looks like you didn't update ril_worker's setupDataCall to use pdprilMessageType (line 1826ish). Maybe a case of overzealous search/replace? :)

Also, if we are changing this property, it should probably be camelCase for consistency

::: dom/system/gonk/ril_worker.js
@@ +2434,4 @@
>          newDataCall.user = newDataCallOptions.user;
>          newDataCall.passwd = newDataCallOptions.passwd;
>          newDataCall.chappap = newDataCallOptions.chappap;
>          newDataCall.pdptype = newDataCallOptions.pdptype;

This would also need to be updated to reflect the pdprilMessageType change.
Comment 3 User image Philipp von Weitershausen [:philikon] 2012-08-06 14:32:20 PDT
Comment 4 User image Ed Morley [:emorley] 2012-08-07 07:36:08 PDT

Note You need to log in before you can comment on or make changes to this bug.