Closed Bug 1453517 Opened 6 years ago Closed 6 years ago

Remove nsIPopupWindowManager

Categories

(Core :: General, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: qdot, Assigned: qdot)

Details

Attachments

(2 files)

nsIPopupWindowManager and nsPopupWindowManager are only used in 2 places in content C++ for perms checks. The XPCOM machinery can be removed, and the checks consolidated to nsContentUtils.
Comment on attachment 8967883 [details]
Bug 1453517 - Consolidate popup permission checks to nsContentUtils;

https://reviewboard.mozilla.org/r/236566/#review242394

::: commit-message-d3bbf:3
(Diff revision 1)
> +Bug 1453517 - Consolidate popup permission checks to nsContentUtils; r=bz
> +
> +Permissions checks for popups were happening in nsIPopUpManager, but

nsIPopupWindowManager.

::: dom/base/nsContentUtils.cpp:11048
(Diff revision 1)
> +  nsCOMPtr<nsIPermissionManager> permissionManager =
> +    services::GetPermissionManager();
> +
> +  if (permissionManager &&
> +      NS_SUCCEEDED(permissionManager->TestPermissionFromPrincipal(aPrincipal, "popup", &permit))) {
> +    // Share some constants between interfaces?

There is not going to be an interface to share with, so you can remove this comment.

::: dom/base/nsContentUtils.cpp:11051
(Diff revision 1)
> +  if (permissionManager &&
> +      NS_SUCCEEDED(permissionManager->TestPermissionFromPrincipal(aPrincipal, "popup", &permit))) {
> +    // Share some constants between interfaces?
> +    if (permit == nsIPermissionManager::ALLOW_ACTION) {
> +      return true;
> +    } else if (permit == nsIPermissionManager::DENY_ACTION) {

Else after return.  Yes, I know you just copied it...
Attachment #8967883 - Flags: review?(bzbarsky) → review+
Comment on attachment 8967884 [details]
Bug 1453517 - Remove nsPopupWindowManager and related XPCOM bits;

https://reviewboard.mozilla.org/r/236568/#review242396

::: extensions/cookie/moz.build:13
(Diff revision 1)
>  
>  UNIFIED_SOURCES += [
>      'nsCookieModule.cpp',
>      'nsCookiePermission.cpp',
>      'nsPermission.cpp',
> -    'nsPermissionManager.cpp',
> +    'nsPermissionManager.cpp'

Please leave the trailing ','
Attachment #8967884 - Flags: review?(bzbarsky) → review+
Pushed by kmachulis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/77694987ba61
Consolidate popup permission checks to nsContentUtils; r=bz
https://hg.mozilla.org/integration/autoland/rev/f154057ddc01
Remove nsPopupWindowManager and related XPCOM bits; r=bz
https://hg.mozilla.org/mozilla-central/rev/77694987ba61
https://hg.mozilla.org/mozilla-central/rev/f154057ddc01
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.