B2G Telephony: Hook up to permissions manager

RESOLVED FIXED in mozilla17

Status

()

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

People

(Reporter: gwagner, Assigned: hsinyi)

Tracking

(Blocks: 1 bug)

unspecified
mozilla17
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:+)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

5 years ago
The whitelist approach will go away pretty soon. We should update to the permissionManager approach.
https://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentUtils.cpp#6834
We should separate this into two bugs, one for telephony, and one for bluetooth..
blocking-basecamp: --- → ?
Dividing telephony and bluetooth into 2 seperate bugs. Bluetooth now Bug 779384
Summary: Hook up telephony and bluetooth to permission API → B2G Telephony: Hook up to permissions manager
(Assignee)

Updated

5 years ago
Assignee: nobody → htsai
(Assignee)

Comment 3

5 years ago
Created attachment 649208 [details] [diff] [review]
Patch part1: use nsIPermissionManager
(Assignee)

Comment 4

5 years ago
Created attachment 649209 [details] [diff] [review]
Patch: testcase
(Assignee)

Comment 5

5 years ago
Created attachment 649212 [details] [diff] [review]
Patch part2: testcase
Attachment #649209 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #649208 - Attachment description: Patch: use nsIPermissionManager → Patch part1: use nsIPermissionManager
Attachment #649208 - Flags: review?(mounir)
Comment on attachment 649208 [details] [diff] [review]
Patch part1: use nsIPermissionManager

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

::: dom/telephony/Telephony.cpp
@@ +562,4 @@
>    NS_ENSURE_SUCCESS(rv, rv);
> +
> +  if (permission == nsIPermissionManager::UNKNOWN_ACTION ||
> +      permission == nsIPermissionManager::DENY_ACTION) {

if (permission != nsIPermissionManager::ALLOW_ACTON) {

@@ +566,5 @@
>      *aTelephony = nullptr;
>      return NS_OK;
>    }
> +  NS_ENSURE_SUCCESS(permission == nsIPermissionManager::ALLOW_ACTION,
> +                    NS_ERROR_FAILURE);

Remove that check.
Attachment #649208 - Flags: review?(mounir) → review+
(Assignee)

Comment 7

5 years ago
Comment on attachment 649212 [details] [diff] [review]
Patch part2: testcase

Hi Philipp, 
Patch part1 passed mounir's review. Could you help review the modifications in telephony testcases? Thanks!
Attachment #649212 - Flags: review?(philipp)
Comment on attachment 649212 [details] [diff] [review]
Patch part2: testcase

Please don't get rid of the clean up. It serves the important purpose of giving each test a clean slate, at least permission-wise. Please use SpecialPowers.removePermission("telephony", document);
Attachment #649212 - Flags: review?(philipp) → review-
(Assignee)

Comment 9

5 years ago
Created attachment 649528 [details] [diff] [review]
Patch part2: testcase (v2)

Updated according to Comment 8.
Attachment #649528 - Flags: review+
Attachment #649212 - Attachment is obsolete: true
(Assignee)

Comment 10

5 years ago
Comment 6 addressed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2898f99f375c

https://hg.mozilla.org/integration/mozilla-inbound/rev/707a4a6340f0
(Assignee)

Comment 11

5 years ago
Created attachment 649544 [details] [diff] [review]
patch (part1) v2

Use nsIPermissionManager. Addressed Comment 6.
Attachment #649208 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/2898f99f375c
https://hg.mozilla.org/mozilla-central/rev/707a4a6340f0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Blocks: 764618
blocking-basecamp: ? → +
You need to log in before you can comment on or make changes to this bug.