tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit ae635f4b7a9a57b14bd7d6c00f80f27534359426
parent a8adba21706f3cc0c935c3bd18fe5f32e6807176
Author: smayya <smayya@mozilla.com>
Date:   Wed,  7 Jan 2026 13:18:31 +0000

Bug 2008859 - Add notifyShown callback to track LNA prompt display state. r=emz,necko-reviewers,jesup

Add a new notifyShown() method to nsIContentPermissionRequest interface
to enable C++ code to be notified when permission prompts are actually
displayed to users. This allows distinguishing between user-interactive
prompts and automatic permission decisions.

Changes:
  - Add notifyShown() to nsIContentPermissionRequest IDL interface
  - Implement notifyShown() in LNAPermissionRequest to record
    local_network_access_prompts_shown telemetry when prompt is shown
  - Update PermissionPromptCallback signature to include bool promptShown
    parameter, allowing nsHttpChannel to differentiate between:
    - prompt_allow/prompt_deny: User saw and responded to prompt
    - auto_allow/auto_deny: Permission automatically granted/denied
  - Add default no-op implementation in ContentPermissionRequestBase for
    other permission types
  - Call notifyShown() from PermissionUI.sys.mjs onBeforeShow() hook

  This enables more accurate telemetry by only recording metrics when
  prompts are actually shown to users, not when permissions are
  automatically granted or denied via temporary permissions.

Differential Revision: https://phabricator.services.mozilla.com/D278069

Diffstat:
Mbrowser/modules/PermissionUI.sys.mjs | 7+++++++
Mdom/base/nsContentPermissionHelper.cpp | 15+++++++++++++++
Mdom/base/nsContentPermissionHelper.h | 1+
Mdom/interfaces/base/nsIContentPermissionPrompt.idl | 7+++++++
Mnetwerk/base/LNAPermissionRequest.cpp | 59++++++++++++++++++++++++++++++++++-------------------------
Mnetwerk/base/LNAPermissionRequest.h | 5++++-
Mnetwerk/protocol/http/nsHttpChannel.cpp | 33+++++++++++++++++++++------------
Mnetwerk/test/browser/browser_test_local_network_access.js | 136+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
8 files changed, 225 insertions(+), 38 deletions(-)

diff --git a/browser/modules/PermissionUI.sys.mjs b/browser/modules/PermissionUI.sys.mjs @@ -1034,6 +1034,13 @@ class LNAPermissionPromptBase extends PermissionPromptForRequest { this.request = request; } + onBeforeShow() { + // Notify LNAPermissionRequest that the prompt is being shown. + // This triggers telemetry recording and notifies nsHttpChannel. + this.request.notifyShown(); + return true; + } + onShown() { this.#startTimeoutTimer(); } diff --git a/dom/base/nsContentPermissionHelper.cpp b/dom/base/nsContentPermissionHelper.cpp @@ -471,6 +471,13 @@ ContentPermissionRequestBase::GetElement(Element** aElement) { } NS_IMETHODIMP +ContentPermissionRequestBase::NotifyShown() { + // Default implementation does nothing. + // Subclasses can override to perform actions when prompt is shown. + return NS_OK; +} + +NS_IMETHODIMP ContentPermissionRequestBase::GetHasValidTransientUserGestureActivation( bool* aHasValidTransientUserGestureActivation) { *aHasValidTransientUserGestureActivation = @@ -759,6 +766,14 @@ nsContentPermissionRequestProxy::GetIsRequestDelegatedToUnsafeThirdParty( } NS_IMETHODIMP +nsContentPermissionRequestProxy::NotifyShown() { + // This is a proxy class that forwards to content process. + // The actual NotifyShown() logic is handled by the real implementation + // in the content process, so we don't need to do anything here. + return NS_OK; +} + +NS_IMETHODIMP nsContentPermissionRequestProxy::Cancel() { if (mParent == nullptr) { return NS_ERROR_FAILURE; diff --git a/dom/base/nsContentPermissionHelper.h b/dom/base/nsContentPermissionHelper.h @@ -105,6 +105,7 @@ class ContentPermissionRequestBase : public nsIContentPermissionRequest { bool* aHasValidTransientUserGestureActivation) override; NS_IMETHOD GetIsRequestDelegatedToUnsafeThirdParty( bool* aIsRequestDelegatedToUnsafeThirdParty) override; + NS_IMETHOD NotifyShown(void) override; // Overrides for Allow() and Cancel() aren't provided by this class. // That is the responsibility of the subclasses. diff --git a/dom/interfaces/base/nsIContentPermissionPrompt.idl b/dom/interfaces/base/nsIContentPermissionPrompt.idl @@ -76,6 +76,13 @@ interface nsIContentPermissionRequest : nsISupports { nsIPrincipal getDelegatePrincipal(in ACString aType); /** + * Notify that the permission prompt has been shown to the user. + * This is called when the prompt UI is displayed, before the user + * makes a decision to allow or deny. + */ + void notifyShown(); + + /** * allow or cancel the request */ [can_run_script] diff --git a/netwerk/base/LNAPermissionRequest.cpp b/netwerk/base/LNAPermissionRequest.cpp @@ -89,7 +89,7 @@ LNAPermissionRequest::GetElement(mozilla::dom::Element** aElement) { NS_IMETHODIMP LNAPermissionRequest::Cancel() { // callback to the http channel on the prompt failure result - mPermissionPromptCallback(false, mType); + mPermissionPromptCallback(false, mType, mPromptWasShown); return NS_OK; } @@ -97,7 +97,39 @@ LNAPermissionRequest::Cancel() { NS_IMETHODIMP LNAPermissionRequest::Allow(JS::Handle<JS::Value> aChoices) { // callback to the http channel on the prompt success result - mPermissionPromptCallback(true, mType); + mPermissionPromptCallback(true, mType, mPromptWasShown); + return NS_OK; +} + +// callback when the permission prompt is shown +NS_IMETHODIMP +LNAPermissionRequest::NotifyShown() { + // Mark that the prompt was shown to the user + mPromptWasShown = true; + + // Record telemetry for permission prompts shown to users + if (mType.Equals(LOCAL_HOST_PERMISSION_KEY)) { + if (mIsRequestDelegatedToUnsafeThirdParty) { + mozilla::glean::networking::local_network_access_prompts_shown + .Get("localhost_cross_site"_ns) + .Add(1); + } else { + mozilla::glean::networking::local_network_access_prompts_shown + .Get("localhost"_ns) + .Add(1); + } + } else if (mType.Equals(LOCAL_NETWORK_PERMISSION_KEY)) { + if (mIsRequestDelegatedToUnsafeThirdParty) { + mozilla::glean::networking::local_network_access_prompts_shown + .Get("local_network_cross_site"_ns) + .Add(1); + } else { + mozilla::glean::networking::local_network_access_prompts_shown + .Get("local_network"_ns) + .Add(1); + } + } + return NS_OK; } @@ -130,29 +162,6 @@ nsresult LNAPermissionRequest::RequestPermission() { return Cancel(); } - // Record telemetry for permission prompts shown to users - if (mType.Equals(LOCAL_HOST_PERMISSION_KEY)) { - if (mIsRequestDelegatedToUnsafeThirdParty) { - mozilla::glean::networking::local_network_access_prompts_shown - .Get("localhost_cross_site"_ns) - .Add(1); - } else { - mozilla::glean::networking::local_network_access_prompts_shown - .Get("localhost"_ns) - .Add(1); - } - } else if (mType.Equals(LOCAL_NETWORK_PERMISSION_KEY)) { - if (mIsRequestDelegatedToUnsafeThirdParty) { - mozilla::glean::networking::local_network_access_prompts_shown - .Get("local_network_cross_site"_ns) - .Add(1); - } else { - mozilla::glean::networking::local_network_access_prompts_shown - .Get("local_network"_ns) - .Add(1); - } - } - if (NS_SUCCEEDED( dom::nsContentPermissionUtils::AskPermission(this, mWindow))) { // Here we could be getting synchronous callback from the prompts depending diff --git a/netwerk/base/LNAPermissionRequest.h b/netwerk/base/LNAPermissionRequest.h @@ -15,7 +15,8 @@ static constexpr nsLiteralCString LOCAL_HOST_PERMISSION_KEY = "localhost"_ns; static constexpr nsLiteralCString LOCAL_NETWORK_PERMISSION_KEY = "local-network"_ns; -using PermissionPromptCallback = std::function<void(bool, const nsACString&)>; +using PermissionPromptCallback = + std::function<void(bool granted, const nsACString& type, bool promptShown)>; /** * Handles permission dialog management for local network accesses @@ -33,6 +34,7 @@ class LNAPermissionRequest final : public dom::ContentPermissionRequestBase { NS_IMETHOD Cancel(void) override; NS_IMETHOD Allow(JS::Handle<JS::Value> choices) override; + NS_IMETHOD NotifyShown(void) override; NS_IMETHOD GetElement(mozilla::dom::Element** aElement) override; nsresult RequestPermission(); @@ -41,6 +43,7 @@ class LNAPermissionRequest final : public dom::ContentPermissionRequestBase { ~LNAPermissionRequest() = default; nsCOMPtr<nsILoadInfo> mLoadInfo; PermissionPromptCallback mPermissionPromptCallback; + bool mPromptWasShown = false; }; } // namespace mozilla::net diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp @@ -8829,20 +8829,36 @@ nsresult nsHttpChannel::ProcessLNAActions() { UpdateLocalNetworkAccessPermissions(permissionKey); if (LNAPermission::Granted == permissionUpdateResult) { - // permission granted + // permission granted (auto-allow via permanent permission) + mLNAPromptAction.AssignLiteral("auto_allow"); return OnPermissionPromptResult(true, permissionKey); } if (LNAPermission::Denied == permissionUpdateResult) { - // permission denied + // permission denied (auto-deny via permanent permission) + mLNAPromptAction.AssignLiteral("auto_deny"); return OnPermissionPromptResult(false, permissionKey); } // If we get here, we don't have any permission to access the local // host/network. We need to prompt the user for action - auto permissionPromptCallback = [self = RefPtr{this}]( - bool aPermissionGranted, - const nsACString& aType) -> void { + auto permissionPromptCallback = + [self = RefPtr{this}](bool aPermissionGranted, const nsACString& aType, + bool aPromptShown) -> void { + // Set mLNAPromptAction based on whether prompt was shown and user response + if (aPromptShown) { + if (aPermissionGranted) { + self->mLNAPromptAction.AssignLiteral("prompt_allow"); + } else { + self->mLNAPromptAction.AssignLiteral("prompt_deny"); + } + } else { + if (aPermissionGranted) { + self->mLNAPromptAction.AssignLiteral("auto_allow"); + } else { + self->mLNAPromptAction.AssignLiteral("auto_deny"); + } + } self->OnPermissionPromptResult(aPermissionGranted, aType); }; @@ -9213,13 +9229,6 @@ nsresult nsHttpChannel::OnPermissionPromptResult(bool aGranted, const nsACString& aType) { mWaitingForLNAPermission = false; - // Set prompt action for telemetry - if (aGranted) { - mLNAPromptAction.AssignLiteral("allow"); - } else { - mLNAPromptAction.AssignLiteral("deny"); - } - if (aGranted) { LOG( ("nsHttpChannel::OnPermissionPromptResult [this=%p] " diff --git a/netwerk/test/browser/browser_test_local_network_access.js b/netwerk/test/browser/browser_test_local_network_access.js @@ -603,3 +603,139 @@ add_task(async function test_lna_prompt_timeout() { await SpecialPowers.popPrefEnv(); }); + +// Test that telemetry is recorded when LNA prompt is shown +// and not incremented for subsequent requests with cached permission +add_task(async function test_lna_prompt_telemetry() { + await restorePermissions(); + + // Reset telemetry + Services.fog.testResetFOG(); + await SpecialPowers.pushPrefEnv({ + set: [["network.lna.address_space.public.override", "127.0.0.1:4443"]], + }); + + const rand1 = Math.random(); + const tab = await BrowserTestUtils.openNewForegroundTab( + gBrowser, + `${baseURL}page_with_non_trackers.html?test=fetch&rand=${rand1}` + ); + + // Wait for the prompt to appear + await BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popupshown"); + + // Verify telemetry was recorded + let metricValue = + await Glean.networking.localNetworkAccessPromptsShown.localhost.testGetValue(); + is(metricValue, 1, "Should record telemetry when localhost prompt is shown"); + + // Grant permission + clickDoorhangerButton( + PROMPT_ALLOW_BUTTON, + gBrowser.selectedBrowser, + "localhost" + ); + + // Wait for permission to be saved + // eslint-disable-next-line mozilla/no-arbitrary-setTimeout + await new Promise(resolve => setTimeout(resolve, 300)); + + // Make a second request in the same tab with cached permission + const rand2 = Math.random(); + const promise = observeAndCheck( + "fetch", + rand2, + Cr.NS_OK, + "Second request should succeed without prompt" + ); + await SpecialPowers.spawn(tab.linkedBrowser, [rand2], async rand => { + await content.fetch(`http://localhost:21555/?type=fetch&rand=${rand}`); + }); + await promise; + + // Verify telemetry was not incremented + metricValue = + await Glean.networking.localNetworkAccessPromptsShown.localhost.testGetValue(); + is( + metricValue, + 1, + "Telemetry should not increment for requests with cached permission" + ); + + gBrowser.removeTab(tab); + await SpecialPowers.popPrefEnv(); +}); + +// Test that telemetry is recorded when user denies LNA prompt +// and not incremented for subsequent requests with temporary deny permission +add_task(async function test_lna_prompt_telemetry_deny() { + await restorePermissions(); + + // Reset telemetry + Services.fog.testResetFOG(); + await SpecialPowers.pushPrefEnv({ + set: [["network.lna.address_space.public.override", "127.0.0.1:4443"]], + }); + + const rand1 = Math.random(); + const promise1 = observeAndCheck( + "fetch", + rand1, + Cr.NS_ERROR_LOCAL_NETWORK_ACCESS_DENIED, + "First request should be denied" + ); + const tab = await BrowserTestUtils.openNewForegroundTab( + gBrowser, + `${baseURL}page_with_non_trackers.html?test=fetch&rand=${rand1}` + ); + + // Wait for the prompt to appear + await BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popupshown"); + + // Verify telemetry was recorded + let metricValue = + await Glean.networking.localNetworkAccessPromptsShown.localhost.testGetValue(); + is(metricValue, 1, "Should record telemetry when localhost prompt is shown"); + + // Deny permission + clickDoorhangerButton( + PROMPT_NOT_NOW_BUTTON, + gBrowser.selectedBrowser, + "localhost" + ); + + await promise1; + + // Wait for permission to be saved + // eslint-disable-next-line mozilla/no-arbitrary-setTimeout + await new Promise(resolve => setTimeout(resolve, 300)); + + // Make a second request - should be auto-denied without showing prompt + // because a temporary deny permission was saved + const rand2 = Math.random(); + const promise2 = observeAndCheck( + "fetch", + rand2, + Cr.NS_ERROR_LOCAL_NETWORK_ACCESS_DENIED, + "Second request should be auto-denied with temporary permission" + ); + await SpecialPowers.spawn(tab.linkedBrowser, [rand2], async rand => { + await content + .fetch(`http://localhost:21555/?type=fetch&rand=${rand}`) + .catch(() => {}); + }); + + await promise2; + + // Verify telemetry was not incremented (no prompt shown with temporary deny) + metricValue = + await Glean.networking.localNetworkAccessPromptsShown.localhost.testGetValue(); + is( + metricValue, + 1, + "Telemetry should not increment for requests with temporary deny permission" + ); + + gBrowser.removeTab(tab); + await SpecialPowers.popPrefEnv(); +});