CCed people as suggested by :ladamski. Single point of permissions check is in dom/camera/DOMCameraManager.cpp::nsDOMCameraManager::Create().

How are we coming along here?

(In reply to Lucas Adamski from comment #3) > How are we coming along here? Planning on finishing up other code this week and getting this done next week.

Add permission check to the camera API. This takes place in nsDOMCameraManager::CheckPermissionAndCreateInstance(). If the check fails, no DOM-facing Camera Manager is created, and no subsequent camera APIs can be called.

Add permission check to the camera API. This takes place in nsDOMCameraManager::CheckPermissionAndCreateInstance(). If the check fails, no DOM-facing Camera Manager is created, and no subsequent camera APIs can be called.

Comment on attachment 661848 [details] [diff] [review] WIP - check for permission to access the camera API Remove duplicate attachment.

:jlebar, would you mind giving this a quick look-over to see if I'm on the right path? I based this patch on the changes in the sub-bugs to 774716, but even when I remove "camera" and "camera-content" from 'permissions' in gaia/apps/camera/manifest.webapp and |./build.sh gaia && ./flash.sh gaia|, the camera always starts.

Comment on attachment 661847 [details] [diff] [review] WIP - check for permission to access the camera API This looks right to me, so if you're having problems, the bug may not be in here. I would also be thrilled if this code, which is now copy-pasted n+1 times, were moved into nsIPermissionManager.

Comment on attachment 662178 [details] [diff] [review] check for permission to access the camera API, refactor similar APIs Also, what was going wrong with your earlier approach? It looks like this diff has two copies of every file (run it through diffstat to see).

A patch without the duplicate files (didn't notice that I was >>'ing instead of >'ing--sorry). The problem is https://bugzilla.mozilla.org/show_bug.cgi?id=785632 , which gives apps the permissions of NO_APP_ID while the plumbing is being finalized. This seems to include permission to access the camera. Right now, removing the hack (in extensions/cookie/nsPermissionManager.cpp) breaks B2G's start-up.

Okay...we also need -U8 in this diff. I have in my ~/.hgrc: [diff] git = 1 showfunc = 1 unified = 8

with -U8

Comment on attachment 662193 [details] [diff] [review] check for permission to access the camera API, refactor similar APIs Thanks a lot for doing this. >diff --git a/netwerk/base/public/nsIPermissionManager.idl b/netwerk/base/public/nsIPermissionManager.idl > [scriptable, uuid(da33450a-f3cb-4fdb-93ee-219644e450c2)] Please change the UUID. (You can use uuidgen, or ask firebot on irc for a uuid with "firebot: uuid"). >@@ -141,16 +142,24 @@ interface nsIPermissionManager : nsISupports > /** > * Test whether the principal has the permission to perform a given action. > * System principals will always have permissions granted. > */ > uint32_t testPermissionFromPrincipal(in nsIPrincipal principal, > in string type); > > /** >+ * Test whether the document associated with this window has permission to >+ * perform a given action. System principals will always have permissions >+ * granted. >+ */ >+ uint32_t testPermissionFromWindow(in nsPIDOMWindow window, >+ in string type); nsPIDOMWindow isn't a scriptable interface; it's defined in a header file, not an IDL file. Therefore you shouldn't use nsPIDOMWindow here, because then we couldn't call this method from JS. nsIDOMWindow should work fine. nsPIDOMWindow inherits from nsIDOMWindow, so you shouldn't even need to modify callers. Also, there's a lack of parallelism in the comment. The first sentence says we're testing whether the /document/ has permission. But the second sentence says that a certain /principal/ always has permission. Either say "Test whether the principal associated with this window's document has permission" or "Documents with the system principal will always have permissions granted." >diff --git a/dom/bluetooth/BluetoothManager.cpp b/dom/bluetooth/BluetoothManager.cpp > nsresult > NS_NewBluetoothManager(nsPIDOMWindow* aWindow, > nsIDOMBluetoothManager** aBluetoothManager) > { > NS_ASSERTION(aWindow, "Null pointer!"); > >- nsPIDOMWindow* innerWindow = aWindow->IsInnerWindow() ? >- aWindow : >- aWindow->GetCurrentInnerWindow(); >- >- // Need the document for security check. >- nsCOMPtr<nsIDocument> document = innerWindow->GetExtantDoc(); >- NS_ENSURE_TRUE(document, NS_NOINTERFACE); >- >- nsCOMPtr<nsIPrincipal> principal = document->NodePrincipal(); >- NS_ENSURE_TRUE(principal, NS_ERROR_UNEXPECTED); >- > nsCOMPtr<nsIPermissionManager> permMgr = > do_GetService(NS_PERMISSIONMANAGER_CONTRACTID); > NS_ENSURE_TRUE(permMgr, NS_ERROR_UNEXPECTED); > > uint32_t permission; > nsresult rv = >- permMgr->TestPermissionFromPrincipal(principal, "mozBluetooth", >+ permMgr->TestPermissionFromWindow(aWindow, "mozBluetooth", > &permission); Please fix the indentation of &permission. >diff --git a/dom/camera/DOMCameraControl.h b/dom/camera/DOMCameraControl.h >--- a/dom/camera/DOMCameraManager.h >+++ b/dom/camera/DOMCameraManager.h >@@ -9,23 +9,25 @@ > > #include "nsCOMPtr.h" > #include "nsAutoPtr.h" > #include "nsIThread.h" > #include "nsThreadUtils.h" > #include "nsIDOMCameraManager.h" > #include "mozilla/Attributes.h" > >+class nsPIDOMWindow; >+ > class nsDOMCameraManager MOZ_FINAL : public nsIDOMCameraManager > { > public: > NS_DECL_ISUPPORTS > NS_DECL_NSIDOMCAMERAMANAGER > >- static already_AddRefed<nsDOMCameraManager> Create(uint64_t aWindowId); >+ static already_AddRefed<nsDOMCameraManager> CheckPermissionAndCreateInstance(nsPIDOMWindow* aWindow); This should be nsIDOMWindow* again. Also, please break the line after the close template. (I see that some other similar factory functions take nsPIDOMWindow, but it's only because they're un-enlightened.) >diff --git a/dom/camera/DOMCameraManager.cpp b/dom/camera/DOMCameraManager.cpp >index 49576a4..60bcd91 100644 >--- a/dom/camera/DOMCameraManager.cpp >+++ b/dom/camera/DOMCameraManager.cpp >@@ -1,12 +1,14 @@ > /* This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this file, > * You can obtain one at http://mozilla.org/MPL/2.0/. */ > >+#include "nsIDocument.h" >+#include "nsIPermissionManager.h" > #include "DOMCameraControl.h" > #include "DOMCameraManager.h" > #include "nsDOMClassInfo.h" > #include "DictionaryHelpers.h" > #include "CameraCommon.h" > > using namespace mozilla; > >@@ -53,26 +55,43 @@ nsDOMCameraManager::~nsDOMCameraManager() > void > nsDOMCameraManager::OnNavigation(uint64_t aWindowId) > { > // TODO: see bug 779145. > } > > // static creator > already_AddRefed<nsDOMCameraManager> >-nsDOMCameraManager::Create(uint64_t aWindowId) >+nsDOMCameraManager::CheckPermissionAndCreateInstance(nsPIDOMWindow* aWindow) > { >- // TODO: see bug 776934. >- > #ifdef PR_LOGGING >+ /** >+ * VERY IMPORTANT: >+ * Setting 'gCameraLog' *MUST* happen before calling any DOM_CAMERA_LOGx() >+ * macros (except _LOGR()--see CameraCommon.h), else you WILL cause >+ * yourself lots of SIGSEGV headachery. >+ */ > if (!gCameraLog) { > gCameraLog = PR_LOG_DEFINE("Camera"); > } This is why everyone else who uses PR_LOG_DEFINE (which, to be fair, is only two other people) uses a static initializer. :) For example, in hal/Hal.cpp: PRLogModuleInfo *sHalLog = PR_LOG_DEFINE("hal"); You don't even have to guard PR_LOG_DEFINE with an ifdef PR_LOGGING; that ifdef is already part of PR_LOG_DEFINE. (See nsprpub/pr/include/prlog.h.) >- nsRefPtr<nsDOMCameraManager> cameraManager = new nsDOMCameraManager(aWindowId); >+ >+ NS_ASSERTION(aWindow, "Null nsPIDOMWindow pointer!"); This assertion is not adding anything. We're going to crash with a null-pointer exception on aWindow at the second-to-last line here, if not sooner. >+ nsCOMPtr<nsIPermissionManager> permMgr = do_GetService(NS_PERMISSIONMANAGER_CONTRACTID); Please wrap this line after the =. >+ NS_ENSURE_TRUE(permMgr, nullptr); >+ >+ uint32_t permission = nsIPermissionManager::DENY_ACTION; >+ permMgr->TestPermissionFromWindow(aWindow, "camera", &permission); >+ if (permission != nsIPermissionManager::ALLOW_ACTION) { >+ printf_stderr("%s:%d : no permission to access camera\n", __func__, __LINE__); This should either go through your gCameraLog or be an NS_WARNING. We do not use printf_stderr for warnings like this, because there's no way to turn it off. >+ return nullptr; >+ } >+ >+ nsRefPtr<nsDOMCameraManager> cameraManager = new nsDOMCameraManager(aWindow->WindowID()); Wrap the line after =, please. >diff --git a/dom/camera/ICameraControl.h b/dom/camera/ICameraControl.h >index 12e6d3d..6fb565e 100644 >--- a/dom/camera/ICameraControl.h >+++ b/dom/camera/ICameraControl.h >@@ -1,17 +1,15 @@ > /* This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this file, > * You can obtain one at http://mozilla.org/MPL/2.0/. */ > > #ifndef DOM_CAMERA_ICAMERACONTROL_H > #define DOM_CAMERA_ICAMERACONTROL_H > >-#include "base/basictypes.h" >-#include "prtypes.h" > #include "jsapi.h" > #include "nsIDOMCameraManager.h" > #include "DictionaryHelpers.h" > #include "CameraCommon.h" > > namespace mozilla { > > using namespace dom; >diff --git a/extensions/cookie/nsPermissionManager.cpp b/extensions/cookie/nsPermissionManager.cpp >index 3258361..b66af07 100644 >--- a/extensions/cookie/nsPermissionManager.cpp >+++ b/extensions/cookie/nsPermissionManager.cpp >@@ -903,16 +903,33 @@ nsPermissionManager::TestPermission(nsIURI *aURI, > nsCOMPtr<nsIPrincipal> principal; > nsresult rv = GetPrincipal(aURI, getter_AddRefs(principal)); > NS_ENSURE_SUCCESS(rv, rv); > > return TestPermissionFromPrincipal(principal, aType, aPermission); > } > > NS_IMETHODIMP >+nsPermissionManager::TestPermissionFromWindow(nsPIDOMWindow* aWindow, >+ const char* aType, >+ uint32_t* aPermission) >+{ >+ nsPIDOMWindow* innerWindow = aWindow->IsInnerWindow() ? >+ aWindow : >+ aWindow->GetCurrentInnerWindow(); >+ >+ // Get the document for security check >+ nsCOMPtr<nsIDocument> document = innerWindow->GetExtantDoc(); >+ NS_ENSURE_TRUE(document, NS_NOINTERFACE); >+ >+ nsCOMPtr<nsIPrincipal> principal = document->NodePrincipal(); >+ return TestPermissionFromPrincipal(principal, "camera", aPermission); >+} >+NS_IMETHODIMP > nsPermissionManager::TestPermissionFromPrincipal(nsIPrincipal* aPrincipal, > const char* aType, > uint32_t* aPermission) We really should make these two methods always return NS_OK and set aPermission to PERMISSION_DENIED by default. That eliminates an entire class of bugs, where someone does uint32_t permission; TestPermissionFromPrincipal(&permission); if (permission != PERMISSION_DENIED) { // Whoa nelly; |permission| may be uninitialized! } You don't have to do that here if you don't want to, though. :) I'd like to have another look after these fixups.

(In reply to Justin Lebar [:jlebar] from comment #15) > Comment on attachment 662193 [details] [diff] [review] > check for permission to access the camera API, refactor similar APIs > > Thanks a lot for doing this. > > > /** > >+ * Test whether the document associated with this window has permission to > >+ * perform a given action. System principals will always have permissions > >+ * granted. > >+ */ > >+ uint32_t testPermissionFromWindow(in nsPIDOMWindow window, > >+ in string type); > > nsPIDOMWindow isn't a scriptable interface; it's defined in a header file, not > an IDL file. Therefore you shouldn't use nsPIDOMWindow here, because then we > couldn't call this method from JS. > > nsIDOMWindow should work fine. nsPIDOMWindow inherits from nsIDOMWindow, so > you shouldn't even need to modify callers. It looks like nsPIDOMWindow doesn't support IsInnerWindow() or GetCurrentInnerWindow(), which are required in TestPermissionFromWindow() to get 'innerWindow'. I suppose I could remove testPermissionFromWindow() from nsIPermissionManager.idl and add it to nsPIDOMWindow instead. > > #ifdef PR_LOGGING > >+ /** > >+ * VERY IMPORTANT: > >+ * Setting 'gCameraLog' *MUST* happen before calling any DOM_CAMERA_LOGx() > >+ * macros (except _LOGR()--see CameraCommon.h), else you WILL cause > >+ * yourself lots of SIGSEGV headachery. > >+ */ > > if (!gCameraLog) { > > gCameraLog = PR_LOG_DEFINE("Camera"); > > } > > >- nsRefPtr<nsDOMCameraManager> cameraManager = new nsDOMCameraManager(aWindowId); > >+ > >+ NS_ASSERTION(aWindow, "Null nsPIDOMWindow pointer!"); > > This assertion is not adding anything. We're going to crash with a > null-pointer exception on aWindow at the second-to-last line here, if not > sooner. This trope appears in lots of other places--should I get rid of them as well? > >diff --git a/dom/camera/ICameraControl.h b/dom/camera/ICameraControl.h > >index 12e6d3d..6fb565e 100644 > >--- a/dom/camera/ICameraControl.h > >+++ b/dom/camera/ICameraControl.h > >@@ -1,17 +1,15 @@ > > /* This Source Code Form is subject to the terms of the Mozilla Public > > * License, v. 2.0. If a copy of the MPL was not distributed with this file, > > * You can obtain one at http://mozilla.org/MPL/2.0/. */ > > > > #ifndef DOM_CAMERA_ICAMERACONTROL_H > > #define DOM_CAMERA_ICAMERACONTROL_H > > > >-#include "base/basictypes.h" > >-#include "prtypes.h" > > #include "jsapi.h" > > #include "nsIDOMCameraManager.h" > > #include "DictionaryHelpers.h" > > #include "CameraCommon.h" > > > > namespace mozilla { > > > > using namespace dom; Just checking: was there supposed to be a comment here? > >diff --git a/extensions/cookie/nsPermissionManager.cpp b/extensions/cookie/nsPermissionManager.cpp > >index 3258361..b66af07 100644 > >--- a/extensions/cookie/nsPermissionManager.cpp > >+++ b/extensions/cookie/nsPermissionManager.cpp > >@@ -903,16 +903,33 @@ nsPermissionManager::TestPermission(nsIURI *aURI, > > nsCOMPtr<nsIPrincipal> principal; > > nsresult rv = GetPrincipal(aURI, getter_AddRefs(principal)); > > NS_ENSURE_SUCCESS(rv, rv); > > > > return TestPermissionFromPrincipal(principal, aType, aPermission); > > } > > > > NS_IMETHODIMP > >+nsPermissionManager::TestPermissionFromWindow(nsPIDOMWindow* aWindow, > >+ const char* aType, > >+ uint32_t* aPermission) > >+{ > >+ nsPIDOMWindow* innerWindow = aWindow->IsInnerWindow() ? > >+ aWindow : > >+ aWindow->GetCurrentInnerWindow(); > >+ > >+ // Get the document for security check > >+ nsCOMPtr<nsIDocument> document = innerWindow->GetExtantDoc(); > >+ NS_ENSURE_TRUE(document, NS_NOINTERFACE); > >+ > >+ nsCOMPtr<nsIPrincipal> principal = document->NodePrincipal(); > >+ return TestPermissionFromPrincipal(principal, "camera", aPermission); > >+} > > >+NS_IMETHODIMP > > nsPermissionManager::TestPermissionFromPrincipal(nsIPrincipal* aPrincipal, > > const char* aType, > > uint32_t* aPermission) > > We really should make these two methods always return NS_OK and set > aPermission to PERMISSION_DENIED by default. That eliminates an entire > class of bugs, where someone does > > uint32_t permission; > TestPermissionFromPrincipal(&permission); > if (permission != PERMISSION_DENIED) { > // Whoa nelly; |permission| may be uninitialized! > } > > You don't have to do that here if you don't want to, though. :) Just grepped through the tree, and there seem to be two usage patterns: (1) uint32_t permission; nsresult rv = TestPermissionFromPrincipal(..., &permission); NS_ENSURE_SUCCESS(rv, rv); if (permission != ALLOW_ACTION) { return; } (2) uint32_t permission = DENY_ACTION; TestPermissionFromPrincipal(..., &permission); if (permission != ALLOW_ACTION) { return; } Places where the permission isn't preloaded AND the return value isn't checked: - dom/src/storage/nsDOMStorage.cpp:1423 - extensions/cookie/nsCookiePermission.cpp:209 File a new bug for these? :) It looks like dom/media/MediaManager.cpp:386 and content/html/content/src/nsHTMLInputElement.cpp:347 both call nsPopupWindowManager::TestPermission(), which preloads the *aPermission from mPolicy. Do you think we would need/want something like that?

(In reply to Mike Habicher [:mikeh] from comment #16) > > It looks like nsPIDOMWindow doesn't support IsInnerWindow() or > GetCurrentInnerWindow(), which are required in TestPermissionFromWindow() to > get 'innerWindow'. I suppose I could remove testPermissionFromWindow() from > nsIPermissionManager.idl and add it to nsPIDOMWindow instead. I just re-read this reply and it makes no sense. :) I meant, I could add it to nsPermissionManager directly, instead of defining it as part of nsIPermissionManager.

> It looks like nsPIDOMWindow doesn't support IsInnerWindow() or GetCurrentInnerWindow(), which are > required in TestPermissionFromWindow() to get 'innerWindow'. Correct. You can QI to nsPIDOMWindow first thing in your implementation. >> This assertion is not adding anything. We're going to crash with a >> null-pointer exception on aWindow at the second-to-last line here, if not >> sooner. > > This trope appears in lots of other places--should I get rid of them as well? Sure if you like, but the more fixes we add to this one patch, the harder it becomes to review. So maybe you can do that in a separate patch. > Just checking: was there supposed to be a comment here? No, you're OK. :) > File a new bug for these? :) Sure, or you can fix them here but in a separate patch (this bug has already morphed into "clean up calls to TestPermissionFrom*"). Whatever you prefer. > Do you think we would need/want something like that? I can't imagine having a policy of anything other than failing closed, so I don't think so.

> I meant, I could add it to nsPermissionManager directly, instead of defining it as part of > nsIPermissionManager. You could, but then every caller would have to static_cast their nsIPermissionManager* to nsPermissionManager*, which is an uncommon idiom in Gecko. Better would be to make the method [noscript] in XPCOM, if we really wanted to use nsPIDOMWindow for some reason.

Incorporate feedback from reviews: - refactor TestPermissionFromPrincipal() calls into TestPermissionFromWindow() - do_QueryInterface() from nsIDOMWindow to nsPIDOMWindow - new UUID on nsIPermissionManager - tidy up the PRLogging - clean ups

Now with extra -U8.

> class nsDOMCameraManager MOZ_FINAL : public nsIDOMCameraManager > { > public: > NS_DECL_ISUPPORTS > NS_DECL_NSIDOMCAMERAMANAGER > >- static already_AddRefed<nsDOMCameraManager> CheckPermissionAndCreateInstance(nsPIDOMWindow* aWindow); >+ static already_AddRefed<nsDOMCameraManager> >+ CheckPermissionAndCreateInstance(nsPIDOMWindow* aWindow); Per the earlier review, this should be nsIDOMWindow*. >diff --git a/dom/camera/DOMCameraManager.cpp b/dom/camera/DOMCameraManager.cpp >+#include "nsDebug.h" I'm surprised this isn't included by one of the other files, but ok! > NS_IMPL_RELEASE(nsDOMCameraManager) > > /** > * Global camera logging object > * > * Set the NSPR_LOG_MODULES environment variable to enable logging > * in a debug build, e.g. NSPR_LOG_MODULES=Camera:5 > */ >-#ifdef PR_LOGGING >-PRLogModuleInfo* gCameraLog; >-#endif >+PRLogModuleInfo* gCameraLog = PR_LOG_DEFINE("Camera");; ;; >@@ -57,60 +56,29 @@ nsDOMCameraManager::OnNavigation(uint64_t aWindowId) > { > // TODO: see bug 779145. > } > > // static creator > already_AddRefed<nsDOMCameraManager> > nsDOMCameraManager::CheckPermissionAndCreateInstance(nsPIDOMWindow* aWindow) >+ permMgr->TestPermissionFromWindow(aWindow, "camera", &permission); > if (permission != nsIPermissionManager::ALLOW_ACTION) { >- printf_stderr("%s:%d : no permission to access camera\n", __func__, __LINE__); >+ NS_WARNING("No permission to access camera\n"); No \n.

(In reply to Justin Lebar [:jlebar] from comment #22) > > class nsDOMCameraManager MOZ_FINAL : public nsIDOMCameraManager > > { > > public: > > NS_DECL_ISUPPORTS > > NS_DECL_NSIDOMCAMERAMANAGER > > > >- static already_AddRefed<nsDOMCameraManager> CheckPermissionAndCreateInstance(nsPIDOMWindow* aWindow); > >+ static already_AddRefed<nsDOMCameraManager> > >+ CheckPermissionAndCreateInstance(nsPIDOMWindow* aWindow); > > Per the earlier review, this should be nsIDOMWindow*. All of the other CheckPermissionAndCreateInstance() functions take an nsPIDOMWindow* as well, not an nsIDOMWindow*. In my particular case, I also need the WindowID() method which only exists on nsPIDOMWindow. Since (unlike TestPermissionFromWindow()) this isn't a scriptable function either, so I'd like to leave this as-is. > > NS_IMPL_RELEASE(nsDOMCameraManager) > > > > /** > > * Global camera logging object > > * > > * Set the NSPR_LOG_MODULES environment variable to enable logging > > * in a debug build, e.g. NSPR_LOG_MODULES=Camera:5 > > */ > >-#ifdef PR_LOGGING > >-PRLogModuleInfo* gCameraLog; > >-#endif > >+PRLogModuleInfo* gCameraLog = PR_LOG_DEFINE("Camera");; > > ;; *sigh* thanks. :)

Incorporate last feedback.

> All of the other CheckPermissionAndCreateInstance() functions take an nsPIDOMWindow* as > well, not an nsIDOMWindow*. Indeed, but that doesn't mean they know what they're doing. :) I'm happy if you want to leave this as-is, but let's not make a habit of adding public APIs (scriptable or otherwise) which take PIDOMWindow; much of the time, it just means that the caller has to do a QI, which is unnecessarily annoying. (If you know your caller will have a PIDOMWindow and you want to /save/ a QI because perf matters, then it might make sense to accept a PIDOMWindow. But that's not the case here.)

If you're ready for this to be checked in, please hg export a patch with a proper commit message and mark the bug as checkin-needed. It looks like you may be familiar with this, but in case you're not, more info is at http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

Now with 100% more headers.

(In reply to Justin Lebar [:jlebar] from comment #25) > > All of the other CheckPermissionAndCreateInstance() functions take an nsPIDOMWindow* as > > well, not an nsIDOMWindow*. > > Indeed, but that doesn't mean they know what they're doing. :) Heh. :) If this is something that you think should be cleaned up across the board, I'll keep it in the back of my mind. Thanks again for the review!

(In reply to Mike Habicher [:mikeh] from comment #27) > Now with 100% more headers. Needs an r=jlebar

Pushed to Try since I don't see any results here. Will checkin if it goes green. https://tbpl.mozilla.org/?tree=Try&rev=f9cf6f65e19f (In reply to Justin Lebar [:jlebar] from comment #29) > Needs an r=jlebar Pfft, the checkin monkeys have that part covered :P

> Pfft, the checkin monkeys have that part covered :P In all seriousness, do we not ask people to put r=foo in their commit messages for checkin-needed bugs anymore? I don't want to be the only one asking for it!

It's preferred, but I also see little point in posting a new patch if all that's being added is the r=. I check every commit message before pushing, so it'll end up on there one way or another.

This is failing tests. https://tbpl.mozilla.org/php/getParsedLog.php?id=15326726&tree=Try 951 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/power/test/test_power_basics.html | Shouldn be able to access power manager with permission.

Fixes a nasty bug that was causing everything to test again camera permissions. New try results: https://tbpl.mozilla.org/?tree=Try&rev=f4e72c50967e

The fix diff: https://bugzilla.mozilla.org/attachment.cgi?oldid=662368&action=interdiff&newid=663084&headers=1

(Now fortified with extra headers.)

...and actually the right attachment this time. *sigh*

The last patch attached appears to have some differences from what was pushed to Try. Pushed again to be safe. I'll land it if it goes green. https://tbpl.mozilla.org/?tree=Try&rev=b6377046bd34

(In reply to Ryan VanderMeulen from comment #38) > https://tbpl.mozilla.org/?tree=Try&rev=b6377046bd34 Green on Try (the Win7 m-oth failure was not from this). https://hg.mozilla.org/integration/mozilla-inbound/rev/da5b2bdfd115