Closed Bug 935361 Opened 8 years ago Closed 8 years ago

Use CheckPermission in the PContentParent Geolocation message handlers

Categories

(Core :: DOM: Geolocation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: reuben, Assigned: reuben)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file)

AssertAppPermission doesn't know how to handle permission requests coming from tabs in the browser app. mozilla::CheckPermission is a reusable and more correct version of the workaround being used in RecvAddGeolocationListener right now.
Attachment #827804 - Flags: review?(doug.turner)
Comment on attachment 827804 [details] [diff] [review]
Use CheckPermission

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

::: dom/ipc/ContentParent.cpp
@@ -3049,5 @@
> -      if (appId == principalAppId) {
> -        found = true;
> -        break;
> -      }
> -    }

Why don't we need this block of code anymore?


I am looking at:
   http://mxr.mozilla.org/mozilla-central/source/dom/ipc/AppProcessChecker.cpp#197
(In reply to Doug Turner (:dougt) from comment #1)
> ::: dom/ipc/ContentParent.cpp
> @@ -3049,5 @@
> > -      if (appId == principalAppId) {
> > -        found = true;
> > -        break;
> > -      }
> > -    }
> 
> Why don't we need this block of code anymore?

We do, see AssertAppPrincipal in the same file. CheckPermission calls it before doing anything else.
Assignee: nobody → reuben.bmo
Attachment #827804 - Flags: review?(doug.turner) → review+
https://hg.mozilla.org/mozilla-central/rev/b57eb03f0abe
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Depends on: 946000
No longer blocks: 955906
Depends on: 951785
Depends on: 955906
No longer depends on: 946000
Whiteboard: [qa-]
Depends on: 975751
You need to log in before you can comment on or make changes to this bug.