ma1 pushed to branch tor-browser-102.15.0esr-12.5-1 at The Tor Project / Applications / Tor Browser Commits: 0db28aff by Andrew Osmond at 2023-08-27T12:55:13+02:00 Bug 1751583. r=media-playback-reviewers,alwu a=RyanVM - - - - - 8cab913f by Tim Huang at 2023-08-27T15:46:35+02:00 Bug 1842030 - Part 1: Adding CanonicalBrowsingContext::IsPrivateBrowsingActive(). r=nika The CanonicalBrowsingContext::IsPrivateBrowsingActive() function allows us to know if private browsing is active. We need it to prevent setting cookies if there is no active private browsing session. Differential Revision: https://phabricator.services.mozilla.com/D184010 - - - - - 0293325a by Tim Huang at 2023-08-27T15:46:35+02:00 Bug 1842030 - Part 2: Check the global private browsing state for HttpBaseChannel::IsBrowsingContextDiscarded() if the loadGroup is not avaiable. r=necko-reviewers,jesup The HttpBaseChannel::IsBrowsingContextDiscarded() did always return false if the loadGroup is not avaiable. But, this may not be correct for the private channels if the private session has been ended. To fix this, we make the function to check the global private browsing state if the loadGroup is not available for private channels. Depends on D184010 Differential Revision: https://phabricator.services.mozilla.com/D184479 - - - - - ee0e3ac2 by hsingh at 2023-08-27T17:55:05+02:00 Bug 1843046: Do not allow notifications in private window. r=saschanaz, a=RyanVM Differential Revision: https://phabricator.services.mozilla.com/D184064 - - - - - 11 changed files: - docshell/base/CanonicalBrowsingContext.cpp - docshell/base/CanonicalBrowsingContext.h - dom/media/gmp/GMPParent.cpp - dom/media/gmp/GMPParent.h - dom/media/gmp/GMPServiceParent.cpp - dom/notification/Notification.cpp - netwerk/protocol/http/HttpBaseChannel.cpp - netwerk/protocol/http/HttpBaseChannel.h - netwerk/protocol/http/HttpChannelParent.cpp - netwerk/test/unit/test_cookiejars.js - netwerk/test/unit_ipc/test_cookiejars_wrap.js Changes: ===================================== docshell/base/CanonicalBrowsingContext.cpp ===================================== @@ -1351,6 +1351,11 @@ uint32_t CanonicalBrowsingContext::CountSiteOrigins( return uniqueSiteOrigins.Count(); } +/* static */ +bool CanonicalBrowsingContext::IsPrivateBrowsingActive() { + return gNumberOfPrivateContexts > 0; +} + void CanonicalBrowsingContext::UpdateMediaControlAction( const MediaControlAction& aAction) { if (IsDiscarded()) { ===================================== docshell/base/CanonicalBrowsingContext.h ===================================== @@ -201,6 +201,9 @@ class CanonicalBrowsingContext final : public BrowsingContext { GlobalObject& aGlobal, const Sequence<mozilla::OwningNonNull<BrowsingContext>>& aRoots); + // Return true if a private browsing session is active. + static bool IsPrivateBrowsingActive(); + // This function would propogate the action to its all child browsing contexts // in content processes. void UpdateMediaControlAction(const MediaControlAction& aAction); ===================================== dom/media/gmp/GMPParent.cpp ===================================== @@ -64,7 +64,7 @@ namespace mozilla::gmp { #define __CLASS__ "GMPParent" GMPParent::GMPParent() - : mState(GMPStateNotLoaded), + : mState(GMPState::NotLoaded), mPluginId(GeckoChildProcessHost::GetUniqueID()), mProcess(nullptr), mDeleteProcessOnlyOnUnload(false), @@ -270,7 +270,7 @@ RefPtr<GenericPromise> GMPParent::Init(GeckoMediaPluginServiceParent* aService, } void GMPParent::Crash() { - if (mState != GMPStateNotLoaded) { + if (mState != GMPState::NotLoaded) { Unused << SendCrashPluginNow(); } } @@ -310,7 +310,7 @@ class NotifyGMPProcessLoadedTask : public Runnable { nsresult GMPParent::LoadProcess() { MOZ_ASSERT(mDirectory, "Plugin directory cannot be NULL!"); MOZ_ASSERT(GMPEventTarget()->IsOnCurrentThread()); - MOZ_ASSERT(mState == GMPStateNotLoaded); + MOZ_ASSERT(mState == GMPState::NotLoaded); nsAutoString path; if (NS_WARN_IF(NS_FAILED(mDirectory->GetPath(path)))) { @@ -388,7 +388,7 @@ nsresult GMPParent::LoadProcess() { GMP_PARENT_LOG_DEBUG("%s: Sent StartPlugin to child process", __FUNCTION__); } - mState = GMPStateLoaded; + mState = GMPState::Loaded; // Hold a self ref while the child process is alive. This ensures that // during shutdown the GMPParent stays alive long enough to @@ -418,8 +418,8 @@ void GMPParent::CloseIfUnused() { MOZ_ASSERT(GMPEventTarget()->IsOnCurrentThread()); GMP_PARENT_LOG_DEBUG("%s", __FUNCTION__); - if ((mDeleteProcessOnlyOnUnload || mState == GMPStateLoaded || - mState == GMPStateUnloading) && + if ((mDeleteProcessOnlyOnUnload || mState == GMPState::Loaded || + mState == GMPState::Unloading) && !IsUsed()) { // Ensure all timers are killed. for (uint32_t i = mTimers.Length(); i > 0; i--) { @@ -437,15 +437,16 @@ void GMPParent::CloseIfUnused() { void GMPParent::CloseActive(bool aDieWhenUnloaded) { MOZ_ASSERT(GMPEventTarget()->IsOnCurrentThread()); - GMP_PARENT_LOG_DEBUG("%s: state %d", __FUNCTION__, mState); + GMP_PARENT_LOG_DEBUG("%s: state %u", __FUNCTION__, + uint32_t(GMPState(mState))); if (aDieWhenUnloaded) { mDeleteProcessOnlyOnUnload = true; // don't allow this to go back... } - if (mState == GMPStateLoaded) { - mState = GMPStateUnloading; + if (mState == GMPState::Loaded) { + mState = GMPState::Unloading; } - if (mState != GMPStateNotLoaded && IsUsed()) { + if (mState != GMPState::NotLoaded && IsUsed()) { Unused << SendCloseActive(); CloseIfUnused(); } @@ -467,7 +468,7 @@ void GMPParent::Shutdown() { } MOZ_ASSERT(!IsUsed()); - if (mState == GMPStateNotLoaded || mState == GMPStateClosing) { + if (mState == GMPState::NotLoaded || mState == GMPState::Closing) { return; } @@ -480,7 +481,7 @@ void GMPParent::Shutdown() { // Destroy ourselves and rise from the fire to save memory mService->ReAddOnGMPThread(self); } // else we've been asked to die and stay dead - MOZ_ASSERT(mState == GMPStateNotLoaded); + MOZ_ASSERT(mState == GMPState::NotLoaded); } class NotifyGMPShutdownTask : public Runnable { @@ -524,10 +525,10 @@ void GMPParent::DeleteProcess() { MOZ_ASSERT(GMPEventTarget()->IsOnCurrentThread()); GMP_PARENT_LOG_DEBUG("%s", __FUNCTION__); - if (mState != GMPStateClosing) { + if (mState != GMPState::Closing) { // Don't Close() twice! // Probably remove when bug 1043671 is resolved - mState = GMPStateClosing; + mState = GMPState::Closing; Close(); } mProcess->Delete(NewRunnableMethod("gmp::GMPParent::ChildTerminated", this, @@ -536,7 +537,7 @@ void GMPParent::DeleteProcess() { mProcess = nullptr; #if defined(MOZ_WIDGET_ANDROID) - if (mState != GMPStateNotLoaded) { + if (mState != GMPState::NotLoaded) { nsCOMPtr<nsIEventTarget> launcherThread(GetIPCLauncher()); MOZ_ASSERT(launcherThread); @@ -553,7 +554,7 @@ void GMPParent::DeleteProcess() { } #endif // defined(MOZ_WIDGET_ANDROID) - mState = GMPStateNotLoaded; + mState = GMPState::NotLoaded; nsCOMPtr<nsIRunnable> r = new NotifyGMPShutdownTask(NS_ConvertUTF8toUTF16(mNodeId)); @@ -623,16 +624,18 @@ bool GMPCapability::Supports(const nsTArray<GMPCapability>& aCapabilities, } bool GMPParent::EnsureProcessLoaded() { - if (mState == GMPStateLoaded) { - return true; - } - if (mState == GMPStateClosing || mState == GMPStateUnloading) { - return false; + switch (mState) { + case GMPState::NotLoaded: + return NS_SUCCEEDED(LoadProcess()); + case GMPState::Loaded: + return true; + case GMPState::Unloading: + case GMPState::Closing: + return false; } - nsresult rv = LoadProcess(); - - return NS_SUCCEEDED(rv); + MOZ_ASSERT_UNREACHABLE("Unhandled GMPState!"); + return false; } void GMPParent::AddCrashAnnotations() { @@ -695,7 +698,7 @@ void GMPParent::ActorDestroy(ActorDestroyReason aWhy) { } // warn us off trying to close again - mState = GMPStateClosing; + mState = GMPState::Closing; mAbnormalShutdownInProgress = true; CloseActive(false); @@ -704,7 +707,7 @@ void GMPParent::ActorDestroy(ActorDestroyReason aWhy) { RefPtr<GMPParent> self(this); // Must not call Close() again in DeleteProcess(), as we'll recurse // infinitely if we do. - MOZ_ASSERT(mState == GMPStateClosing); + MOZ_ASSERT(mState == GMPState::Closing); DeleteProcess(); // Note: final destruction will be Dispatched to ourself mService->ReAddOnGMPThread(self); ===================================== dom/media/gmp/GMPParent.h ===================================== @@ -20,6 +20,7 @@ #include "nsString.h" #include "nsTArray.h" #include "nsIFile.h" +#include "mozilla/Atomics.h" #include "mozilla/MozPromise.h" namespace mozilla::gmp { @@ -42,12 +43,7 @@ class GMPCapability { const nsCString& aAPI, const nsCString& aTag); }; -enum GMPState { - GMPStateNotLoaded, - GMPStateLoaded, - GMPStateUnloading, - GMPStateClosing -}; +enum class GMPState : uint32_t { NotLoaded, Loaded, Unloading, Closing }; class GMPContentParent; @@ -194,7 +190,7 @@ class GMPParent final uint32_t& aArchSet); #endif - GMPState mState; + Atomic<GMPState> mState; nsCOMPtr<nsIFile> mDirectory; // plugin directory on disk nsString mName; // base name of plugin on disk, UTF-16 because used for paths nsCString mDisplayName; // name of plugin displayed to users ===================================== dom/media/gmp/GMPServiceParent.cpp ===================================== @@ -643,7 +643,7 @@ void GeckoMediaPluginServiceParent::SendFlushFOGData( MutexAutoLock lock(mMutex); for (const RefPtr<GMPParent>& gmp : mPlugins) { - if (gmp->State() != GMPState::GMPStateLoaded) { + if (gmp->State() != GMPState::Loaded) { // Plugins that are not in the Loaded state have no process attached to // them, and any IPC we would attempt to send them would be ignored (or // result in a warning on debug builds). @@ -681,7 +681,7 @@ GeckoMediaPluginServiceParent::TestTriggerMetrics() { { MutexAutoLock lock(mMutex); for (const RefPtr<GMPParent>& gmp : mPlugins) { - if (gmp->State() != GMPState::GMPStateLoaded) { + if (gmp->State() != GMPState::Loaded) { // Plugins that are not in the Loaded state have no process attached to // them, and any IPC we would attempt to send them would be ignored (or // result in a warning on debug builds). @@ -1003,7 +1003,7 @@ void GeckoMediaPluginServiceParent::RemoveOnGMPThread( } RefPtr<GMPParent> gmp = mPlugins[i]; - if (aDeleteFromDisk && gmp->State() != GMPStateNotLoaded) { + if (aDeleteFromDisk && gmp->State() != GMPState::NotLoaded) { // We have to wait for the child process to release its lib handle // before we can delete the GMP. inUse = true; @@ -1014,7 +1014,7 @@ void GeckoMediaPluginServiceParent::RemoveOnGMPThread( } } - if (gmp->State() == GMPStateNotLoaded || !aCanDefer) { + if (gmp->State() == GMPState::NotLoaded || !aCanDefer) { // GMP not in use or shutdown is being forced; can shut it down now. deadPlugins.AppendElement(gmp); mPlugins.RemoveElementAt(i); ===================================== dom/notification/Notification.cpp ===================================== @@ -478,6 +478,9 @@ NotificationPermissionRequest::Run() { bool blocked = false; if (isSystem) { mPermission = NotificationPermission::Granted; + } else if (mPrincipal->GetPrivateBrowsingId() != 0) { + mPermission = NotificationPermission::Denied; + blocked = true; } else { // File are automatically granted permission. @@ -1483,7 +1486,12 @@ already_AddRefed<Promise> Notification::RequestPermission( aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } + nsCOMPtr<nsIPrincipal> principal = sop->GetPrincipal(); + if (!principal) { + aRv.Throw(NS_ERROR_UNEXPECTED); + return nullptr; + } RefPtr<Promise> promise = Promise::Create(window->AsGlobal(), aRv); if (aRv.Failed()) { @@ -1537,6 +1545,15 @@ NotificationPermission Notification::GetPermissionInternal(nsISupports* aGlobal, } nsCOMPtr<nsIPrincipal> principal = sop->GetPrincipal(); + if (!principal) { + aRv.Throw(NS_ERROR_UNEXPECTED); + return NotificationPermission::Denied; + } + + if (principal->GetPrivateBrowsingId() != 0) { + return NotificationPermission::Denied; + } + return GetPermissionInternal(principal, aRv); } ===================================== netwerk/protocol/http/HttpBaseChannel.cpp ===================================== @@ -2390,7 +2390,23 @@ void HttpBaseChannel::NotifySetCookie(const nsACString& aCookie) { } bool HttpBaseChannel::IsBrowsingContextDiscarded() const { - return mLoadGroup && mLoadGroup->GetIsBrowsingContextDiscarded(); + // If there is no loadGroup attached to the current channel, we check the + // global private browsing state for the private channel instead. For + // non-private channel, we will always return false here. + // + // Note that we can only access the global private browsing state in the + // parent process. So, we will fallback to just return false in the content + // process. + if (!mLoadGroup) { + if (!XRE_IsParentProcess()) { + return false; + } + + return mLoadInfo->GetOriginAttributes().mPrivateBrowsingId != 0 && + !dom::CanonicalBrowsingContext::IsPrivateBrowsingActive(); + } + + return mLoadGroup->GetIsBrowsingContextDiscarded(); } // https://mikewest.github.io/corpp/#process-navigation-response ===================================== netwerk/protocol/http/HttpBaseChannel.h ===================================== @@ -619,6 +619,7 @@ class HttpBaseChannel : public nsHashPropertyBag, friend class PrivateBrowsingChannel<HttpBaseChannel>; friend class InterceptFailedOnStop; + friend class HttpChannelParent; protected: // this section is for main-thread-only object ===================================== netwerk/protocol/http/HttpChannelParent.cpp ===================================== @@ -18,6 +18,7 @@ #include "mozilla/net/NeckoParent.h" #include "mozilla/InputStreamLengthHelper.h" #include "mozilla/IntegerPrintfMacros.h" +#include "mozilla/Preferences.h" #include "mozilla/ProfilerLabels.h" #include "mozilla/StoragePrincipalHelper.h" #include "mozilla/UniquePtr.h" @@ -2005,6 +2006,17 @@ void HttpChannelParent::SetCookie(nsCString&& aCookie) { LOG(("HttpChannelParent::SetCookie [this=%p]", this)); MOZ_ASSERT(!mAfterOnStartRequestBegun); MOZ_ASSERT(mCookie.IsEmpty()); + + // The loadGroup of the channel in the parent process could be null in the + // XPCShell content process test, see test_cookiejars_wrap.js. In this case, + // we cannot explicitly set the loadGroup for the parent channel because it's + // created from the content process. To workaround this, we add a testing pref + // to skip this check. + if (!Preferences::GetBool( + "network.cookie.skip_browsing_context_check_in_parent_for_testing") && + mChannel->IsBrowsingContextDiscarded()) { + return; + } mCookie = std::move(aCookie); } ===================================== netwerk/test/unit/test_cookiejars.js ===================================== @@ -60,6 +60,23 @@ function setupChannel(path) { }); chan.loadInfo.originAttributes = tests[i].originAttributes; chan.QueryInterface(Ci.nsIHttpChannel); + + let loadGroup = Cc["@mozilla.org/network/load-group;1"].createInstance( + Ci.nsILoadGroup + ); + + if (chan.loadInfo.originAttributes.privateBrowsingId == 0) { + loadGroup.notificationCallbacks = Cu.createLoadContext(); + chan.loadGroup = loadGroup; + + chan.notificationCallbacks = Cu.createLoadContext(); + } else { + loadGroup.notificationCallbacks = Cu.createPrivateLoadContext(); + chan.loadGroup = loadGroup; + + chan.notificationCallbacks = Cu.createPrivateLoadContext(); + } + return chan; } ===================================== netwerk/test/unit_ipc/test_cookiejars_wrap.js ===================================== @@ -4,6 +4,10 @@ function run_test() { "network.cookieJarSettings.unblocked_for_testing", true ); + Services.prefs.setBoolPref( + "network.cookie.skip_browsing_context_check_in_parent_for_testing", + true + ); Services.prefs.setIntPref("network.cookie.cookieBehavior", 0); run_test_in_child("../unit/test_cookiejars.js"); } View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/8bd8142... -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/8bd8142... You're receiving this email because of your account on gitlab.torproject.org.