ma1 pushed to branch base-browser-115.11.0esr-13.5-1 at The Tor Project / Applications / Tor Browser

Commits: 8584ab91 by Kershaw Chang at 2024-05-11T00:21:58+02:00 Bug 1892449 - Set network.http.digest_auth_cnonce_length to 16, a=dmeehan

Apparently, setting this value to 64 breaks some sites. We should use the same length as Chrome.

Original Revision: https://phabricator.services.mozilla.com/D208103

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

- - - - - 328452de by Nika Layzell at 2024-05-11T00:22:00+02:00 Bug 1875248 - Check for network error preventing ExternalHelperAppService before DONT_RETARGET, r=smaug

This reverts the change from 30cde47f9364e5c7da78fd08fa8ab21737d22399, and instead re-orders the NS_ERROR_FILE_NOT_FOUND check before DONT_RETARGET.

Testing suggests that a-download-click-404.html behaviour isn't impacted, and this improves the handling of this edge-case when doing process switching.

Differential Revision: https://phabricator.services.mozilla.com/D202007 - - - - - a593aa4b by Jonathan Kew at 2024-05-11T00:22:01+02:00 Bug 1886598 - Struct with Pointer member may not be memmove-able. r=gfx-reviewers,lsalzman

Differential Revision: https://phabricator.services.mozilla.com/D206633 - - - - - 45188fff by alwu at 2024-05-11T00:22:02+02:00 Bug 1644383 - add mutexs to avoid data race. r=media-playback-reviewers,padenot

Differential Revision: https://phabricator.services.mozilla.com/D206943 - - - - -

7 changed files:

- dom/media/ipc/RemoteMediaDataDecoder.cpp - dom/media/ipc/RemoteMediaDataDecoder.h - dom/media/platforms/wrappers/MediaChangeMonitor.cpp - dom/media/platforms/wrappers/MediaChangeMonitor.h - gfx/thebes/gfxPlatformFontList.h - modules/libpref/init/StaticPrefList.yaml - uriloader/base/nsURILoader.cpp

Changes:

===================================== dom/media/ipc/RemoteMediaDataDecoder.cpp ===================================== @@ -18,7 +18,12 @@ namespace mozilla { ##__VA_ARGS__)

RemoteMediaDataDecoder::RemoteMediaDataDecoder(RemoteDecoderChild* aChild) - : mChild(aChild) { + : mChild(aChild), + mDescription("RemoteMediaDataDecoder"_ns), + mProcessName("unknown"_ns), + mCodecName("unknown"_ns), + mIsHardwareAccelerated(false), + mConversion(ConversionRequired::kNeedNone) { LOG("%p is created", this); }

@@ -48,6 +53,7 @@ RefPtrMediaDataDecoder::InitPromise RemoteMediaDataDecoder::Init() { ->Then( RemoteDecoderManagerChild::GetManagerThread(), __func__, [self, this](TrackType aTrack) { + MutexAutoLock lock(mMutex); // If shutdown has started in the meantime shutdown promise may // be resloved before this task. In this case mChild will be null // and the init promise has to be canceled. @@ -127,6 +133,7 @@ RefPtr<ShutdownPromise> RemoteMediaDataDecoder::Shutdown() {

bool RemoteMediaDataDecoder::IsHardwareAccelerated( nsACString& aFailureReason) const { + MutexAutoLock lock(mMutex); aFailureReason = mHardwareAcceleratedReason; return mIsHardwareAccelerated; } @@ -145,18 +152,24 @@ void RemoteMediaDataDecoder::SetSeekThreshold(const media::TimeUnit& aTime) {

MediaDataDecoder::ConversionRequired RemoteMediaDataDecoder::NeedsConversion() const { + MutexAutoLock lock(mMutex); return mConversion; }

nsCString RemoteMediaDataDecoder::GetDescriptionName() const { + MutexAutoLock lock(mMutex); return mDescription; }

nsCString RemoteMediaDataDecoder::GetProcessName() const { + MutexAutoLock lock(mMutex); return mProcessName; }

-nsCString RemoteMediaDataDecoder::GetCodecName() const { return mCodecName; } +nsCString RemoteMediaDataDecoder::GetCodecName() const { + MutexAutoLock lock(mMutex); + return mCodecName; +}

#undef LOG

===================================== dom/media/ipc/RemoteMediaDataDecoder.h ===================================== @@ -53,14 +53,16 @@ class RemoteMediaDataDecoder final // destructor when we can guarantee no other threads are accessing it). Only // read from the manager thread. RefPtr<RemoteDecoderChild> mChild; + + mutable Mutex mMutex{"RemoteMediaDataDecoder"}; + // Only ever written/modified during decoder initialisation. - // As such can be accessed from any threads after that. - nsCString mDescription = "RemoteMediaDataDecoder"_ns; - nsCString mProcessName = "unknown"_ns; - nsCString mCodecName = "unknown"_ns; - bool mIsHardwareAccelerated = false; - nsCString mHardwareAcceleratedReason; - ConversionRequired mConversion = ConversionRequired::kNeedNone; + nsCString mDescription MOZ_GUARDED_BY(mMutex); + nsCString mProcessName MOZ_GUARDED_BY(mMutex); + nsCString mCodecName MOZ_GUARDED_BY(mMutex); + bool mIsHardwareAccelerated MOZ_GUARDED_BY(mMutex); + nsCString mHardwareAcceleratedReason MOZ_GUARDED_BY(mMutex); + ConversionRequired mConversion MOZ_GUARDED_BY(mMutex); };

} // namespace mozilla

===================================== dom/media/platforms/wrappers/MediaChangeMonitor.cpp ===================================== @@ -668,6 +668,7 @@ RefPtr<ShutdownPromise> MediaChangeMonitor::ShutdownDecoder() { AssertOnThread(); mConversionRequired.reset(); if (mDecoder) { + MutexAutoLock lock(mMutex); RefPtr<MediaDataDecoder> decoder = std::move(mDecoder); return decoder->Shutdown(); } @@ -715,6 +716,7 @@ MediaChangeMonitor::CreateDecoder() { ->Then( GetCurrentSerialEventTarget(), __func__, [self = RefPtr{this}, this](RefPtr<MediaDataDecoder>&& aDecoder) { + MutexAutoLock lock(mMutex); mDecoder = std::move(aDecoder); DDLINKCHILD("decoder", mDecoder.get()); return CreateDecoderPromise::CreateAndResolve(true, __func__); @@ -963,6 +965,11 @@ void MediaChangeMonitor::FlushThenShutdownDecoder( ->Track(mFlushRequest); }

+MediaDataDecoder* MediaChangeMonitor::GetDecoderOnNonOwnerThread() const { + MutexAutoLock lock(mMutex); + return mDecoder; +} + #undef LOG

} // namespace mozilla

===================================== dom/media/platforms/wrappers/MediaChangeMonitor.h ===================================== @@ -41,34 +41,34 @@ class MediaChangeMonitor final RefPtr<ShutdownPromise> Shutdown() override; bool IsHardwareAccelerated(nsACString& aFailureReason) const override; nsCString GetDescriptionName() const override { - if (mDecoder) { - return mDecoder->GetDescriptionName(); + if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) { + return decoder->GetDescriptionName(); } return "MediaChangeMonitor decoder (pending)"_ns; } nsCString GetProcessName() const override { - if (mDecoder) { - return mDecoder->GetProcessName(); + if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) { + return decoder->GetProcessName(); } return "MediaChangeMonitor"_ns; } nsCString GetCodecName() const override { - if (mDecoder) { - return mDecoder->GetCodecName(); + if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) { + return decoder->GetCodecName(); } return "MediaChangeMonitor"_ns; } void SetSeekThreshold(const media::TimeUnit& aTime) override; bool SupportDecoderRecycling() const override { - if (mDecoder) { - return mDecoder->SupportDecoderRecycling(); + if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) { + return decoder->SupportDecoderRecycling(); } return false; }

ConversionRequired NeedsConversion() const override { - if (mDecoder) { - return mDecoder->NeedsConversion(); + if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) { + return decoder->NeedsConversion(); } // Default so no conversion is performed. return ConversionRequired::kNeedNone; @@ -97,6 +97,9 @@ class MediaChangeMonitor final MOZ_ASSERT(!mThread || mThread->IsOnCurrentThread()); }

+ // This is used for getting decoder debug info on other threads. Thread-safe. + MediaDataDecoder* GetDecoderOnNonOwnerThread() const; + bool CanRecycleDecoder() const;

typedef MozPromise<bool, MediaResult, true /* exclusive */> @@ -137,6 +140,13 @@ class MediaChangeMonitor final const CreateDecoderParamsForAsync mParams; // Keep any seek threshold set for after decoder creation and initialization. Maybemedia::TimeUnit mPendingSeekThreshold; + + // This lock is used for mDecoder specifically, but it doens't need to be used + // for every places accessing mDecoder which is mostly on the owner thread. + // However, when requesting decoder debug info, it can happen on other + // threads, so we need this mutex to avoid the data race of + // creating/destroying decoder and accessing decoder's debug info. + mutable Mutex MOZ_ANNOTATED mMutex{"MediaChangeMonitor"}; };

} // namespace mozilla

===================================== gfx/thebes/gfxPlatformFontList.h ===================================== @@ -124,7 +124,7 @@ class ShmemCharMapHashEntry final : public PLDHashEntryHdr { return aCharMap->GetChecksum(); }

- enum { ALLOW_MEMMOVE = true }; + enum { ALLOW_MEMMOVE = false }; // because of the Pointer member

private: // charMaps are stored in the shared memory that FontList objects point to,

===================================== modules/libpref/init/StaticPrefList.yaml ===================================== @@ -12779,6 +12779,18 @@ value: true mirror: always

+ # The length of cnonce string used in HTTP digest auth. +- name: network.http.digest_auth_cnonce_length + type: uint32_t + value: 16 + mirror: always + + # If true, HTTP response content-type headers will be parsed using the standards-compliant MimeType parser +- name: network.standard_content_type_parsing.response_headers + type: RelaxedAtomicBool + value: true + mirror: always + # The maximum count that we allow socket prrocess to crash. If this count is # reached, we won't use networking over socket process. - name: network.max_socket_process_failed_count

===================================== uriloader/base/nsURILoader.cpp ===================================== @@ -414,28 +414,28 @@ nsresult nsDocumentOpenInfo::DispatchContent(nsIRequest* request) { NS_ASSERTION(!m_targetStreamListener, "If we found a listener, why are we not using it?");

- if (mFlags & nsIURILoader::DONT_RETARGET) { - LOG( - (" External handling forced or (listener not interested and no " - "stream converter exists), and retargeting disallowed -> aborting")); - return NS_ERROR_WONT_HANDLE_CONTENT; - } - // Before dispatching to the external helper app service, check for an HTTP // error page. If we got one, we don't want to handle it with a helper app, // really. - // The WPT a-download-click-404.html requires us to silently handle this - // without displaying an error page, so we just return early here. - // See bug 1604308 for discussion around what the ideal behaviour is. nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(request)); if (httpChannel) { bool requestSucceeded; rv = httpChannel->GetRequestSucceeded(&requestSucceeded); if (NS_FAILED(rv) || !requestSucceeded) { - return NS_OK; + LOG( + (" Returning NS_ERROR_FILE_NOT_FOUND from " + "nsDocumentOpenInfo::DispatchContent due to failed HTTP response")); + return NS_ERROR_FILE_NOT_FOUND; } }

+ if (mFlags & nsIURILoader::DONT_RETARGET) { + LOG( + (" External handling forced or (listener not interested and no " + "stream converter exists), and retargeting disallowed -> aborting")); + return NS_ERROR_WONT_HANDLE_CONTENT; + } + // Fifth step: // // All attempts to dispatch this content have failed. Just pass it off to

View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/d698749...