From 54b2e445779bfdba50b792f6bc8bde0c94d178ad Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 30 Oct 2018 07:48:55 +0000 Subject: [PATCH] Bug 1502747 - Remove nsIContentPermissionType.access and all of its supporting code r=snorp,baku This field was originally added for the b2g-only DeviceStorage API, and isn't used for anything else right now. This reverts the remaining parts of bug 1043136 and bug 1043136 as well as some support code for mobile. Differential Revision: https://phabricator.services.mozilla.com/D10014 --HG-- extra : moz-landing-system : lando --- dom/base/nsContentPermissionHelper.cpp | 18 ++-------------- dom/base/nsContentPermissionHelper.h | 3 --- dom/geolocation/nsGeolocation.cpp | 1 - dom/html/AutoplayPermissionRequest.cpp | 1 - .../base/nsIContentPermissionPrompt.idl | 6 ------ dom/ipc/PContentPermission.ipdlh | 1 - dom/midi/MIDIPermissionRequest.cpp | 1 - dom/notification/Notification.cpp | 1 - dom/push/Push.js | 1 - dom/quota/StorageManager.cpp | 1 - .../components/ContentPermissionPrompt.js | 21 ++++++------------- .../geckoview/GeckoViewPermission.js | 1 - .../geckoview/test/PermissionDelegateTest.kt | 10 ++++----- .../mozilla/geckoview/test/util/Callbacks.kt | 2 +- .../org/mozilla/geckoview/GeckoSession.java | 7 ++----- .../geckoview_example/GeckoViewActivity.java | 3 +-- 16 files changed, 16 insertions(+), 62 deletions(-) diff --git a/dom/base/nsContentPermissionHelper.cpp b/dom/base/nsContentPermissionHelper.cpp index a7fef6948fc1..5590fda960a8 100644 --- a/dom/base/nsContentPermissionHelper.cpp +++ b/dom/base/nsContentPermissionHelper.cpp @@ -211,11 +211,9 @@ ContentPermissionRequestParent::IsBeingDestroyed() NS_IMPL_ISUPPORTS(ContentPermissionType, nsIContentPermissionType) ContentPermissionType::ContentPermissionType(const nsACString& aType, - const nsACString& aAccess, const nsTArray& aOptions) { mType = aType; - mAccess = aAccess; mOptions = aOptions; } @@ -230,13 +228,6 @@ ContentPermissionType::GetType(nsACString& aType) return NS_OK; } -NS_IMETHODIMP -ContentPermissionType::GetAccess(nsACString& aAccess) -{ - aAccess = mAccess; - return NS_OK; -} - NS_IMETHODIMP ContentPermissionType::GetOptions(nsIArray** aOptions) { @@ -276,7 +267,6 @@ nsContentPermissionUtils::ConvertPermissionRequestToArray(nsTArray cpt = new ContentPermissionType(aSrcArray[i].type(), - aSrcArray[i].access(), aSrcArray[i].options()); aDesArray->AppendElement(cpt); } @@ -292,9 +282,7 @@ nsContentPermissionUtils::ConvertArrayToPermissionRequest(nsIArray* aSrcArray, for (uint32_t i = 0; i < len; i++) { nsCOMPtr cpt = do_QueryElementAt(aSrcArray, i); nsAutoCString type; - nsAutoCString access; cpt->GetType(type); - cpt->GetAccess(access); nsCOMPtr optionArray; cpt->GetOptions(getter_AddRefs(optionArray)); @@ -312,7 +300,7 @@ nsContentPermissionUtils::ConvertArrayToPermissionRequest(nsIArray* aSrcArray, } } - aDesArray.AppendElement(PermissionRequest(type, access, options)); + aDesArray.AppendElement(PermissionRequest(type, options)); } return len; } @@ -335,14 +323,12 @@ ContentPermissionRequestChildMap() /* static */ nsresult nsContentPermissionUtils::CreatePermissionArray(const nsACString& aType, - const nsACString& aAccess, const nsTArray& aOptions, nsIArray** aTypesArray) { nsCOMPtr types = do_CreateInstance(NS_ARRAY_CONTRACTID); RefPtr permType = new ContentPermissionType(aType, - aAccess, - aOptions); + aOptions); types->AppendElement(permType); types.forget(aTypesArray); diff --git a/dom/base/nsContentPermissionHelper.h b/dom/base/nsContentPermissionHelper.h index b85e9bd7c851..435dee0435f2 100644 --- a/dom/base/nsContentPermissionHelper.h +++ b/dom/base/nsContentPermissionHelper.h @@ -48,14 +48,12 @@ public: NS_DECL_NSICONTENTPERMISSIONTYPE ContentPermissionType(const nsACString& aType, - const nsACString& aAccess, const nsTArray& aOptions); protected: virtual ~ContentPermissionType(); nsCString mType; - nsCString mAccess; nsTArray mOptions; }; @@ -72,7 +70,6 @@ public: static nsresult CreatePermissionArray(const nsACString& aType, - const nsACString& aAccess, const nsTArray& aOptions, nsIArray** aTypesArray); diff --git a/dom/geolocation/nsGeolocation.cpp b/dom/geolocation/nsGeolocation.cpp index 32c4ea1c4b90..8add3dc6f46d 100644 --- a/dom/geolocation/nsGeolocation.cpp +++ b/dom/geolocation/nsGeolocation.cpp @@ -303,7 +303,6 @@ nsGeolocationRequest::GetTypes(nsIArray** aTypes) { nsTArray emptyOptions; return nsContentPermissionUtils::CreatePermissionArray(NS_LITERAL_CSTRING("geolocation"), - NS_LITERAL_CSTRING("unused"), emptyOptions, aTypes); } diff --git a/dom/html/AutoplayPermissionRequest.cpp b/dom/html/AutoplayPermissionRequest.cpp index d9be79e0987e..068381fb0f19 100644 --- a/dom/html/AutoplayPermissionRequest.cpp +++ b/dom/html/AutoplayPermissionRequest.cpp @@ -56,7 +56,6 @@ AutoplayPermissionRequest::GetTypes(nsIArray** aTypes) nsTArray emptyOptions; return dom::nsContentPermissionUtils::CreatePermissionArray( NS_LITERAL_CSTRING("autoplay-media"), - NS_LITERAL_CSTRING("unused"), emptyOptions, aTypes); } diff --git a/dom/interfaces/base/nsIContentPermissionPrompt.idl b/dom/interfaces/base/nsIContentPermissionPrompt.idl index 0c7d5c85c3da..e1a2fa61f123 100644 --- a/dom/interfaces/base/nsIContentPermissionPrompt.idl +++ b/dom/interfaces/base/nsIContentPermissionPrompt.idl @@ -21,12 +21,6 @@ interface nsIContentPermissionType : nsISupports { */ readonly attribute ACString type; - /** - * The access of the permission request, such as - * "read". - */ - readonly attribute ACString access; - /** * The array of available options. */ diff --git a/dom/ipc/PContentPermission.ipdlh b/dom/ipc/PContentPermission.ipdlh index 1909c0ef843f..a70b5bca9496 100644 --- a/dom/ipc/PContentPermission.ipdlh +++ b/dom/ipc/PContentPermission.ipdlh @@ -7,7 +7,6 @@ namespace dom { struct PermissionRequest { nsCString type; - nsCString access; nsString[] options; }; diff --git a/dom/midi/MIDIPermissionRequest.cpp b/dom/midi/MIDIPermissionRequest.cpp index 624da266e6a3..8b1419246675 100644 --- a/dom/midi/MIDIPermissionRequest.cpp +++ b/dom/midi/MIDIPermissionRequest.cpp @@ -69,7 +69,6 @@ MIDIPermissionRequest::GetTypes(nsIArray** aTypes) options.AppendElement(NS_LITERAL_STRING("sysex")); } return nsContentPermissionUtils::CreatePermissionArray(NS_LITERAL_CSTRING("midi"), - NS_LITERAL_CSTRING("unused"), options, aTypes); } diff --git a/dom/notification/Notification.cpp b/dom/notification/Notification.cpp index dd24bd1dc537..16ae26934b1b 100644 --- a/dom/notification/Notification.cpp +++ b/dom/notification/Notification.cpp @@ -680,7 +680,6 @@ NotificationPermissionRequest::GetTypes(nsIArray** aTypes) { nsTArray emptyOptions; return nsContentPermissionUtils::CreatePermissionArray(NS_LITERAL_CSTRING("desktop-notification"), - NS_LITERAL_CSTRING("unused"), emptyOptions, aTypes); } diff --git a/dom/push/Push.js b/dom/push/Push.js index 3c7f8d907657..cc722da6ae82 100644 --- a/dom/push/Push.js +++ b/dom/push/Push.js @@ -180,7 +180,6 @@ Push.prototype = { // Create an array with a single nsIContentPermissionType element. let type = { type: "desktop-notification", - access: null, options: [], QueryInterface: ChromeUtils.generateQI([Ci.nsIContentPermissionType]), }; diff --git a/dom/quota/StorageManager.cpp b/dom/quota/StorageManager.cpp index 8e8ff19452be..a8dbb4f39425 100644 --- a/dom/quota/StorageManager.cpp +++ b/dom/quota/StorageManager.cpp @@ -814,7 +814,6 @@ PersistentStoragePermissionRequest::GetTypes(nsIArray** aTypes) return nsContentPermissionUtils::CreatePermissionArray( NS_LITERAL_CSTRING("persistent-storage"), - NS_LITERAL_CSTRING("unused"), emptyOptions, aTypes); } diff --git a/mobile/android/components/ContentPermissionPrompt.js b/mobile/android/components/ContentPermissionPrompt.js index 6c97b3326880..0ba2ca76131d 100644 --- a/mobile/android/components/ContentPermissionPrompt.js +++ b/mobile/android/components/ContentPermissionPrompt.js @@ -17,12 +17,6 @@ const kEntities = { "geolocation": "geolocation", }; -// For these types, prompt for permission if action is unknown. -const PROMPT_FOR_UNKNOWN = [ - "desktop-notification", - "geolocation", -]; - function ContentPermissionPrompt() {} ContentPermissionPrompt.prototype = { @@ -30,7 +24,7 @@ ContentPermissionPrompt.prototype = { QueryInterface: ChromeUtils.generateQI([Ci.nsIContentPermissionPrompt]), - handleExistingPermission: function handleExistingPermission(request, type, denyUnknown, callback) { + handleExistingPermission: function handleExistingPermission(request, type, isApp, callback) { let result = Services.perms.testExactPermissionFromPrincipal(request.principal, type); if (result == Ci.nsIPermissionManager.ALLOW_ACTION) { callback(/* allow */ true); @@ -42,7 +36,7 @@ ContentPermissionPrompt.prototype = { return true; } - if (denyUnknown && result == Ci.nsIPermissionManager.UNKNOWN_ACTION) { + if (isApp && result == Ci.nsIPermissionManager.UNKNOWN_ACTION) { callback(/* allow */ false); return true; } @@ -92,10 +86,7 @@ ContentPermissionPrompt.prototype = { }; // Returns true if the request was handled - let access = (perm.access && perm.access !== "unused") ? - (perm.type + "-" + perm.access) : perm.type; - if (this.handleExistingPermission(request, access, - /* denyUnknown */ isApp || !PROMPT_FOR_UNKNOWN.includes(perm.type), callback)) { + if (this.handleExistingPermission(request, perm.type, isApp, callback)) { return; } @@ -107,7 +98,7 @@ ContentPermissionPrompt.prototype = { callback: function(aChecked) { // If the user checked "Don't ask again" or this is a desktopNotification, make a permanent exception if (aChecked || entityName == "desktopNotification2") - Services.perms.addFromPrincipal(request.principal, access, Ci.nsIPermissionManager.DENY_ACTION); + Services.perms.addFromPrincipal(request.principal, perm.type, Ci.nsIPermissionManager.DENY_ACTION); callback(/* allow */ false); }, @@ -117,10 +108,10 @@ ContentPermissionPrompt.prototype = { callback: function(aChecked) { // If the user checked "Don't ask again" or this is a desktopNotification, make a permanent exception if (aChecked || entityName == "desktopNotification2") { - Services.perms.addFromPrincipal(request.principal, access, Ci.nsIPermissionManager.ALLOW_ACTION); + Services.perms.addFromPrincipal(request.principal, perm.type, Ci.nsIPermissionManager.ALLOW_ACTION); } else if (isApp) { // Otherwise allow the permission for the current session if the request comes from an app - Services.perms.addFromPrincipal(request.principal, access, Ci.nsIPermissionManager.ALLOW_ACTION, Ci.nsIPermissionManager.EXPIRE_SESSION); + Services.perms.addFromPrincipal(request.principal, perm.type, Ci.nsIPermissionManager.ALLOW_ACTION, Ci.nsIPermissionManager.EXPIRE_SESSION); } callback(/* allow */ true); diff --git a/mobile/android/components/geckoview/GeckoViewPermission.js b/mobile/android/components/geckoview/GeckoViewPermission.js index ac24545a50d5..fabd26ac4678 100644 --- a/mobile/android/components/geckoview/GeckoViewPermission.js +++ b/mobile/android/components/geckoview/GeckoViewPermission.js @@ -205,7 +205,6 @@ GeckoViewPermission.prototype = { type: "GeckoView:ContentPermission", uri: aRequest.principal.URI.displaySpec, perm: perm.type, - access: perm.access !== "unused" ? perm.access : null, }).then(granted => { if (!granted) { return false; diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PermissionDelegateTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PermissionDelegateTest.kt index d9cc3b367787..520ee78dbade 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PermissionDelegateTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PermissionDelegateTest.kt @@ -127,11 +127,10 @@ class PermissionDelegateTest : BaseSessionTest() { mainSession.delegateDuringNextWait(object : Callbacks.PermissionDelegate { // Ensure the content permission is asked first, before the Android permission. @AssertCalled(count = 1, order = [1]) - override fun onContentPermissionRequest(session: GeckoSession, uri: String, type: Int, access: String?, callback: GeckoSession.PermissionDelegate.Callback) { + override fun onContentPermissionRequest(session: GeckoSession, uri: String, type: Int, callback: GeckoSession.PermissionDelegate.Callback) { assertThat("URI should match", uri, endsWith(HELLO_HTML_PATH)) assertThat("Type should match", type, equalTo(GeckoSession.PermissionDelegate.PERMISSION_GEOLOCATION)) - assertThat("Access should be null", access, nullValue()) callback.grant() } @@ -159,7 +158,7 @@ class PermissionDelegateTest : BaseSessionTest() { mainSession.delegateDuringNextWait(object : Callbacks.PermissionDelegate { @AssertCalled(count = 1) - override fun onContentPermissionRequest(session: GeckoSession, uri: String, type: Int, access: String?, callback: GeckoSession.PermissionDelegate.Callback) { + override fun onContentPermissionRequest(session: GeckoSession, uri: String, type: Int, callback: GeckoSession.PermissionDelegate.Callback) { callback.reject() } @@ -182,11 +181,10 @@ class PermissionDelegateTest : BaseSessionTest() { mainSession.delegateDuringNextWait(object : Callbacks.PermissionDelegate { @AssertCalled(count = 1) - override fun onContentPermissionRequest(session: GeckoSession, uri: String, type: Int, access: String?, callback: GeckoSession.PermissionDelegate.Callback) { + override fun onContentPermissionRequest(session: GeckoSession, uri: String, type: Int, callback: GeckoSession.PermissionDelegate.Callback) { assertThat("URI should match", uri, endsWith(HELLO_HTML_PATH)) assertThat("Type should match", type, equalTo(GeckoSession.PermissionDelegate.PERMISSION_DESKTOP_NOTIFICATION)) - assertThat("Access should be null", access, nullValue()) callback.grant() } }) @@ -204,7 +202,7 @@ class PermissionDelegateTest : BaseSessionTest() { mainSession.delegateDuringNextWait(object : Callbacks.PermissionDelegate { @AssertCalled(count = 1) - override fun onContentPermissionRequest(session: GeckoSession, uri: String, type: Int, access: String?, callback: GeckoSession.PermissionDelegate.Callback) { + override fun onContentPermissionRequest(session: GeckoSession, uri: String, type: Int, callback: GeckoSession.PermissionDelegate.Callback) { callback.reject() } }) diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/util/Callbacks.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/util/Callbacks.kt index 1a17438c768e..30d25d2791c8 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/util/Callbacks.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/util/Callbacks.kt @@ -75,7 +75,7 @@ class Callbacks private constructor() { callback.reject() } - override fun onContentPermissionRequest(session: GeckoSession, uri: String, type: Int, access: String?, callback: GeckoSession.PermissionDelegate.Callback) { + override fun onContentPermissionRequest(session: GeckoSession, uri: String, type: Int, callback: GeckoSession.PermissionDelegate.Callback) { callback.reject() } diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java index 644997cde215..993e909d4e39 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java @@ -521,8 +521,7 @@ public class GeckoSession extends LayerSession } delegate.onContentPermissionRequest( GeckoSession.this, message.getString("uri"), - type, message.getString("access"), - new PermissionCallback(typeString, callback)); + type, new PermissionCallback(typeString, callback)); } else if ("GeckoView:MediaPermission".equals(event)) { GeckoBundle[] videoBundles = message.getBundleArray("video"); GeckoBundle[] audioBundles = message.getBundleArray("audio"); @@ -3317,12 +3316,10 @@ public class GeckoSession extends LayerSession * PERMISSION_GEOLOCATION * PERMISSION_DESKTOP_NOTIFICATION * PERMISSION_AUTOPLAY_MEDIA - * @param access Not used. * @param callback Callback interface. */ void onContentPermissionRequest(GeckoSession session, String uri, - @Permission int type, - String access, Callback callback); + @Permission int type, Callback callback); class MediaSource { @IntDef({SOURCE_CAMERA, SOURCE_SCREEN, SOURCE_APPLICATION, diff --git a/mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java b/mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java index d3feef237e62..a07112931a25 100644 --- a/mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java +++ b/mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java @@ -545,8 +545,7 @@ public class GeckoViewActivity extends AppCompatActivity { @Override public void onContentPermissionRequest(final GeckoSession session, final String uri, - final int type, final String access, - final Callback callback) { + final int type, final Callback callback) { final int resId; if (PERMISSION_GEOLOCATION == type) { resId = R.string.request_geolocation;