Last Comment Bug 779353 - B2G Telephony: Hook up to permissions manager
: B2G Telephony: Hook up to permissions manager
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla17
Assigned To: Hsin-Yi Tsai [:hsinyi]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 774716 764618
  Show dependency treegraph
 
Reported: 2012-07-31 16:32 PDT by Gregor Wagner [:gwagner]
Modified: 2012-08-08 10:48 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
Patch part1: use nsIPermissionManager (2.78 KB, patch)
2012-08-06 01:03 PDT, Hsin-Yi Tsai [:hsinyi]
mounir: review+
Details | Diff | Splinter Review
Patch: testcase (15.78 KB, patch)
2012-08-06 01:04 PDT, Hsin-Yi Tsai [:hsinyi]
no flags Details | Diff | Splinter Review
Patch part2: testcase (15.81 KB, patch)
2012-08-06 01:15 PDT, Hsin-Yi Tsai [:hsinyi]
philipp: review-
Details | Diff | Splinter Review
Patch part2: testcase (v2) (13.15 KB, patch)
2012-08-06 20:24 PDT, Hsin-Yi Tsai [:hsinyi]
philipp: review+
Details | Diff | Splinter Review
patch (part1) v2 (2.50 KB, patch)
2012-08-06 22:58 PDT, Hsin-Yi Tsai [:hsinyi]
no flags Details | Diff | Splinter Review

Description Gregor Wagner [:gwagner] 2012-07-31 16:32:05 PDT
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
Comment 1 Marshall Culpepper [:marshall_law] 2012-07-31 18:07:16 PDT
We should separate this into two bugs, one for telephony, and one for bluetooth..
Comment 2 Kyle Machulis [:qdot] 2012-07-31 18:25:55 PDT
Dividing telephony and bluetooth into 2 seperate bugs. Bluetooth now Bug 779384
Comment 3 Hsin-Yi Tsai [:hsinyi] 2012-08-06 01:03:36 PDT
Created attachment 649208 [details] [diff] [review]
Patch part1: use nsIPermissionManager
Comment 4 Hsin-Yi Tsai [:hsinyi] 2012-08-06 01:04:04 PDT
Created attachment 649209 [details] [diff] [review]
Patch: testcase
Comment 5 Hsin-Yi Tsai [:hsinyi] 2012-08-06 01:15:45 PDT
Created attachment 649212 [details] [diff] [review]
Patch part2: testcase
Comment 6 Mounir Lamouri (:mounir) 2012-08-06 08:40:59 PDT
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.
Comment 7 Hsin-Yi Tsai [:hsinyi] 2012-08-06 19:25:49 PDT
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!
Comment 8 Philipp von Weitershausen [:philikon] 2012-08-06 19:33:43 PDT
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);
Comment 9 Hsin-Yi Tsai [:hsinyi] 2012-08-06 20:24:03 PDT
Created attachment 649528 [details] [diff] [review]
Patch part2: testcase (v2)

Updated according to Comment 8.
Comment 11 Hsin-Yi Tsai [:hsinyi] 2012-08-06 22:58:30 PDT
Created attachment 649544 [details] [diff] [review]
patch (part1) v2

Use nsIPermissionManager. Addressed Comment 6.

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