Contacts API: Shortcut permission check in the child

RESOLVED FIXED in Firefox 23

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: gwagner, Assigned: gwagner)

Tracking

unspecified
mozilla23
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(blocking-b2g:leo+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

Details

(Whiteboard: [fixed-in-birch])

Attachments

(1 attachment, 4 obsolete attachments)

Assignee

Description

6 years ago
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.
Assignee

Comment 1

6 years ago
Posted patch patch (obsolete) — Splinter Review
Assignee

Comment 2

6 years ago
Posted patch patch (obsolete) — Splinter Review
Attachment #737517 - Attachment is obsolete: true
Assignee

Comment 3

6 years ago
Posted patch patch (obsolete) — Splinter Review
Attachment #737527 - Attachment is obsolete: true
Assignee

Updated

6 years ago
Attachment #737533 - Flags: review?(reuben.bmo)
Attachment #737533 - Flags: review?(jonas)
Assignee

Updated

6 years ago
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+
Assignee

Comment 5

6 years ago
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+
Assignee

Comment 7

6 years ago
Posted patch patch (obsolete) — Splinter Review
Attachment #737533 - Attachment is obsolete: true
Attachment #738432 - Flags: review+
Assignee

Comment 8

6 years ago
Posted patch patchSplinter Review
Attachment #738432 - Attachment is obsolete: true
Attachment #738433 - Flags: review+
Assignee

Updated

6 years ago
Whiteboard: checkin-needed
https://hg.mozilla.org/projects/birch/rev/4f58c0e74b97
Whiteboard: checkin-needed → [fixed-in-birch]
Assignee

Updated

6 years ago
Blocks: 857398
blocking-b2g: --- → tef?
Assignee

Updated

6 years ago
Blocks: 847399
https://hg.mozilla.org/mozilla-central/rev/4f58c0e74b97
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Blocks a leo+ bug so blocking.
blocking-b2g: tef? → leo+
Flags: needinfo?(anygregor)
Assignee

Comment 13

6 years ago
(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.