Closed Bug 909786 Opened 7 years ago Closed 7 years ago

Use incremental number as the hash key of |controlCallbacks| in NetworkManager::controlMessage()

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hchang, Assigned: hchang)

Details

Attachments

(1 file, 2 obsolete files)

In NetworkManager.js, when invoking controlMessage(params, callback), the name of callback function would be used as the hash key of |controlCallbacks|. It's unsafe since it's difficult to guarantee all callers using distinct function names.

There is a simple solution already used in WifiManager.js, which is using an incremental |id| as the hash key and delete after callback.
Summary: Use incremental id as the hash keys of |controlCallbacks| in NetworkManager::controlMessage() → Use incremental numbers as the hash keys of |controlCallbacks| in NetworkManager::controlMessage()
Summary: Use incremental numbers as the hash keys of |controlCallbacks| in NetworkManager::controlMessage() → Use incremental number as the hash key of |controlCallbacks| in NetworkManager::controlMessage()
Attached patch Bug909786.patch (obsolete) — Splinter Review
Assignee: nobody → hchang
Attachment #796071 - Flags: review?(vchang)
Oh oh I found something wrong in the patch. I'll attach another patch for review later on.
Comment on attachment 796071 [details] [diff] [review]
Bug909786.patch

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

Remove from review queue per comment 2.
Attachment #796071 - Flags: review?(vchang)
Attached patch Bug909786.patch (obsolete) — Splinter Review
Attachment #796071 - Attachment is obsolete: true
Comment on attachment 797633 [details] [diff] [review]
Bug909786.patch

Verified locally and on Try server
https://tbpl.mozilla.org/?tree=Try&rev=dda8c080b9d3
Attachment #797633 - Flags: review?(vchang)
Comment on attachment 797633 [details] [diff] [review]
Bug909786.patch

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

Looks good. Thank you.
Attachment #797633 - Flags: review?(vchang) → review+
Attachment #797633 - Attachment is obsolete: true
Attachment #801602 - Attachment description: Bug909786.patch (added r=vchang) → Bug909786.patch (has been r+. added r=vchang)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/687e3a6868bf
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.