Closed Bug 861905 Opened 8 years ago Closed 8 years ago

Contacts API: Shortcut permission check in the child

Categories

(Core :: DOM: Device Interfaces, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23
blocking-b2g leo+
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix

People

(Reporter: gwagner, Assigned: gwagner)

References

Details

(Whiteboard: [fixed-in-birch])

Attachments

(1 file, 4 obsolete files)

We spend a lot of time checking the permission for the call log and in the sms app. We can avoid one child-parent roundtrip per DB entry.
Attached patch patch (obsolete) — Splinter Review
Attached patch patch (obsolete) — Splinter Review
Attachment #737517 - Attachment is obsolete: true
Attached patch patch (obsolete) — Splinter Review
Attachment #737527 - Attachment is obsolete: true
Attachment #737533 - Flags: review?(reuben.bmo)
Attachment #737533 - Flags: review?(jonas)
Assignee: nobody → anygregor
Comment on attachment 737533 [details] [diff] [review]
patch

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

Nice!

::: dom/contacts/ContactManager.js
@@ +562,5 @@
>        default:
>          access = "unknown";
>        }
>  
> +    // Shortcut for ALLOW_ACTION

Can you expand this explanation a little bit? Something like
// Avoid the IPC round trip if we don't have to prompt for the permission.
Attachment #737533 - Flags: review?(reuben.bmo) → review+
Comment on attachment 737533 [details] [diff] [review]
patch

Maybe Mounir has more time for a review.
Attachment #737533 - Flags: review?(mounir)
Comment on attachment 737533 [details] [diff] [review]
patch

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

r=me but it's not crystal clear to me why we were not doing that since the beginning. APIs should simply check for the permission on the child and anyway, the parent will kill them if the child request something that they should not be requesting.
Attachment #737533 - Flags: review?(mounir)
Attachment #737533 - Flags: review?(jonas)
Attachment #737533 - Flags: review+
Attached patch patch (obsolete) — Splinter Review
Attachment #737533 - Attachment is obsolete: true
Attachment #738432 - Flags: review+
Attached patch patchSplinter Review
Attachment #738432 - Attachment is obsolete: true
Attachment #738433 - Flags: review+
Whiteboard: checkin-needed
https://hg.mozilla.org/projects/birch/rev/4f58c0e74b97
Whiteboard: checkin-needed → [fixed-in-birch]
Blocks: 857398
blocking-b2g: --- → tef?
Blocks: 847399
https://hg.mozilla.org/mozilla-central/rev/4f58c0e74b97
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Blocks a leo+ bug so blocking.
blocking-b2g: tef? → leo+
Flags: needinfo?(anygregor)
(In reply to David Scravaglieri [:scravag] from comment #11)
> Blocks a leo+ bug so blocking.

I guess everything is done here.
Flags: needinfo?(anygregor)
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.