From 344e75f55653fc82a6298ca7503982ff1f2ae41d Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Wed, 28 Jun 2017 00:00:30 +0200 Subject: [PATCH] Backed out changeset 976a4789deea (bug 1344751) --- netwerk/base/nsStandardURL.cpp | 49 ++++++--------------------- netwerk/base/nsStandardURL.h | 4 +-- netwerk/dns/nsIDNService.cpp | 61 +--------------------------------- netwerk/dns/nsIDNService.h | 16 --------- 4 files changed, 13 insertions(+), 117 deletions(-) diff --git a/netwerk/base/nsStandardURL.cpp b/netwerk/base/nsStandardURL.cpp index 3bf863b20638..0a53105ba382 100644 --- a/netwerk/base/nsStandardURL.cpp +++ b/netwerk/base/nsStandardURL.cpp @@ -23,7 +23,6 @@ #include "mozilla/MemoryReporting.h" #include "mozilla/ipc/URIUtils.h" #include -#include "mozilla/SyncRunnable.h" #include "nsContentUtils.h" #include "prprf.h" #include "nsReadableUtils.h" @@ -43,9 +42,7 @@ static LazyLogModule gStandardURLLog("nsStandardURL"); #ifdef MOZ_RUST_URLPARSE #include "RustURL.h" - -// Modified on the main thread, read on both main thread and worker threads. -Atomic nsStandardURL::gRustEnabled(false); +bool nsStandardURL::gRustEnabled = false; // Fall back to CPP-parsed URLs if the Rust one doesn't match. #define MOZ_RUST_URLPARSE_FALLBACK @@ -140,16 +137,9 @@ namespace net { static NS_DEFINE_CID(kThisImplCID, NS_THIS_STANDARDURL_IMPL_CID); static NS_DEFINE_CID(kStandardURLCID, NS_STANDARDURL_CID); -// This will always be initialized and destroyed on the main thread, but -// can be safely used on other threads. nsIIDNService *nsStandardURL::gIDN = nullptr; - -// This value will only be updated on the main thread once. Worker threads -// may race when reading this values, but that's OK because in the worst -// case we will just dispatch a noop runnable to the main thread. bool nsStandardURL::gInitialized = false; - -const char nsStandardURL::gHostLimitDigits[] = { '/', '\\', '?', '#', 0 }; +char nsStandardURL::gHostLimitDigits[] = { '/', '\\', '?', '#', 0 }; // Invalid host characters // We still allow % because it is in the ID of addons. @@ -193,8 +183,6 @@ nsPrefObserver::Observe(nsISupports *subject, const char *topic, const char16_t *data) { - MOZ_ASSERT(NS_IsMainThread()); - if (!strcmp(topic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID)) { nsCOMPtr prefBranch( do_QueryInterface(subject) ); if (prefBranch) { @@ -313,10 +301,8 @@ nsStandardURL::nsStandardURL(bool aSupportsFileURL, bool aTrackURL) { LOG(("Creating nsStandardURL @%p\n", this)); - // gInitialized changes value only once (false->true) on the main thread. - // It's OK to race here because in the worst case we'll just - // dispatch a noop runnable to the main thread. if (!gInitialized) { + gInitialized = true; InitGlobalObjects(); } @@ -369,19 +355,6 @@ DumpLeakedURLs::~DumpLeakedURLs() void nsStandardURL::InitGlobalObjects() { - if (!NS_IsMainThread()) { - RefPtr r = NS_NewRunnableFunction(&nsStandardURL::InitGlobalObjects); - SyncRunnable::DispatchToThread(GetMainThreadEventTarget(), r, false); - return; - } - - if (gInitialized) { - return; - } - - MOZ_ASSERT(NS_IsMainThread()); - gInitialized = true; - nsCOMPtr prefBranch( do_GetService(NS_PREFSERVICE_CONTRACTID) ); if (prefBranch) { nsCOMPtr obs( new nsPrefObserver() ); @@ -390,18 +363,11 @@ nsStandardURL::InitGlobalObjects() #endif PrefsChanged(prefBranch, nullptr); } - - nsCOMPtr serv(do_GetService(NS_IDNSERVICE_CONTRACTID)); - if (serv) { - NS_ADDREF(gIDN = serv.get()); - MOZ_ASSERT(gIDN); - } } void nsStandardURL::ShutdownGlobalObjects() { - MOZ_ASSERT(NS_IsMainThread()); NS_IF_RELEASE(gIDN); #ifdef DEBUG_DUMP_URLS_AT_SHUTDOWN @@ -683,6 +649,13 @@ nsStandardURL::NormalizeIDN(const nsACString& host, nsCString& result) NS_ASSERTION(mHostEncoding == eEncoding_ASCII, "unexpected default encoding"); bool isASCII; + if (!gIDN) { + nsCOMPtr serv(do_GetService(NS_IDNSERVICE_CONTRACTID)); + if (serv) { + NS_ADDREF(gIDN = serv.get()); + } + } + result.Truncate(); nsresult rv = NS_ERROR_UNEXPECTED; if (gIDN) { @@ -1276,8 +1249,6 @@ nsStandardURL::WriteSegment(nsIBinaryOutputStream *stream, const URLSegment &seg /* static */ void nsStandardURL::PrefsChanged(nsIPrefBranch *prefs, const char *pref) { - MOZ_ASSERT(NS_IsMainThread()); - LOG(("nsStandardURL::PrefsChanged [pref=%s]\n", pref)); #define PREF_CHANGED(p) ((pref == nullptr) || !strcmp(pref, p)) diff --git a/netwerk/base/nsStandardURL.h b/netwerk/base/nsStandardURL.h index f061c5c9a764..a8aeb18c1f12 100644 --- a/netwerk/base/nsStandardURL.h +++ b/netwerk/base/nsStandardURL.h @@ -298,7 +298,7 @@ private: // global objects. don't use COMPtr as its destructor will cause a // coredump if we leak it. static nsIIDNService *gIDN; - static const char gHostLimitDigits[]; + static char gHostLimitDigits[]; static bool gInitialized; public: @@ -307,7 +307,7 @@ public: #endif #ifdef MOZ_RUST_URLPARSE - static Atomic gRustEnabled; + static bool gRustEnabled; RefPtr mRustURL; #endif }; diff --git a/netwerk/dns/nsIDNService.cpp b/netwerk/dns/nsIDNService.cpp index 365f182d17d9..d0830fdf7026 100644 --- a/netwerk/dns/nsIDNService.cpp +++ b/netwerk/dns/nsIDNService.cpp @@ -63,9 +63,6 @@ NS_IMPL_ISUPPORTS(nsIDNService, nsresult nsIDNService::Init() { - MOZ_ASSERT(NS_IsMainThread()); - MutexAutoLock lock(mLock); - nsCOMPtr prefs(do_GetService(NS_PREFSERVICE_CONTRACTID)); if (prefs) prefs->GetBranch(NS_NET_PREF_IDNWHITELIST, getter_AddRefs(mIDNWhitelistPrefBranch)); @@ -86,9 +83,6 @@ NS_IMETHODIMP nsIDNService::Observe(nsISupports *aSubject, const char *aTopic, const char16_t *aData) { - MOZ_ASSERT(NS_IsMainThread()); - MutexAutoLock lock(mLock); - if (!strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID)) { nsCOMPtr prefBranch( do_QueryInterface(aSubject) ); if (prefBranch) @@ -99,9 +93,6 @@ NS_IMETHODIMP nsIDNService::Observe(nsISupports *aSubject, void nsIDNService::prefsChanged(nsIPrefBranch *prefBranch, const char16_t *pref) { - MOZ_ASSERT(NS_IsMainThread()); - mLock.AssertCurrentThreadOwns(); - if (!pref || NS_LITERAL_STRING(NS_NET_PREF_IDNBLACKLIST).Equals(pref)) { nsCOMPtr blacklist; nsresult rv = prefBranch->GetComplexValue(NS_NET_PREF_IDNBLACKLIST, @@ -140,12 +131,9 @@ void nsIDNService::prefsChanged(nsIPrefBranch *prefBranch, const char16_t *pref) } nsIDNService::nsIDNService() - : mLock("DNService pref value lock") - , mShowPunycode(false) + : mShowPunycode(false) , mIDNUseWhitelist(false) { - MOZ_ASSERT(NS_IsMainThread()); - #ifdef IDNA2008 uint32_t IDNAOptions = UIDNA_CHECK_BIDI | UIDNA_CHECK_CONTEXTJ; if (!kIDNA2008_TransitionalProcessing) { @@ -164,8 +152,6 @@ nsIDNService::nsIDNService() nsIDNService::~nsIDNService() { - MOZ_ASSERT(NS_IsMainThread()); - #ifdef IDNA2008 uidna_close(mIDNA); #else @@ -395,41 +381,8 @@ NS_IMETHODIMP nsIDNService::Normalize(const nsACString & input, return NS_OK; } -namespace { - -class MOZ_STACK_CLASS MutexSettableAutoUnlock final -{ - Mutex* mMutex; -public: - MutexSettableAutoUnlock() - : mMutex(nullptr) - { } - - void - Acquire(mozilla::Mutex& aMutex) - { - MOZ_ASSERT(!mMutex); - mMutex = &aMutex; - mMutex->Lock(); - } - - ~MutexSettableAutoUnlock() - { - if (mMutex) { - mMutex->Unlock(); - } - } -}; - -} // anonymous namespace - NS_IMETHODIMP nsIDNService::ConvertToDisplayIDN(const nsACString & input, bool * _isASCII, nsACString & _retval) { - MutexSettableAutoUnlock lock; - if (!NS_IsMainThread()) { - lock.Acquire(mLock); - } - // If host is ACE, then convert to UTF-8 if the host is in the IDN whitelist. // Else, if host is already UTF-8, then make sure it is normalized per IDN. @@ -801,10 +754,6 @@ nsresult nsIDNService::decodeACE(const nsACString& in, nsACString& out, bool nsIDNService::isInWhitelist(const nsACString &host) { - if (!NS_IsMainThread()) { - mLock.AssertCurrentThreadOwns(); - } - if (mIDNUseWhitelist && mIDNWhitelistPrefBranch) { nsAutoCString tld(host); // make sure the host is ACE for lookup and check that there are no @@ -831,10 +780,6 @@ bool nsIDNService::isInWhitelist(const nsACString &host) bool nsIDNService::isLabelSafe(const nsAString &label) { - if (!NS_IsMainThread()) { - mLock.AssertCurrentThreadOwns(); - } - if (!isOnlySafeChars(PromiseFlatString(label), mIDNBlacklist)) { return false; } @@ -978,10 +923,6 @@ static const int32_t scriptComboTable[13][9] = { bool nsIDNService::illegalScriptCombo(Script script, int32_t& savedScript) { - if (!NS_IsMainThread()) { - mLock.AssertCurrentThreadOwns(); - } - if (savedScript == -1) { savedScript = findScriptIndex(script); return false; diff --git a/netwerk/dns/nsIDNService.h b/netwerk/dns/nsIDNService.h index d0487f7c0ecd..19aa94da055b 100644 --- a/netwerk/dns/nsIDNService.h +++ b/netwerk/dns/nsIDNService.h @@ -168,25 +168,12 @@ private: idn_nameprep_t mNamePrepHandle; nsCOMPtr mNormalizer; #endif - - // We use this mutex to guard access to: - // |mIDNBlacklist|, |mShowPunycode|, |mRestrictionProfile|, - // |mIDNUseWhitelist|. - // - // These members can only be updated on the main thread and - // read on any thread. Therefore, acquiring the mutex is required - // only for threads other than the main thread. - mozilla::Mutex mLock; - - // guarded by mLock nsXPIDLString mIDNBlacklist; /** * Flag set by the pref network.IDN_show_punycode. When it is true, * IDNs containing non-ASCII characters are always displayed to the * user in punycode - * - * guarded by mLock */ bool mShowPunycode; @@ -200,11 +187,8 @@ private: eHighlyRestrictiveProfile, eModeratelyRestrictiveProfile }; - // guarded by mLock; restrictionProfile mRestrictionProfile; - // guarded by mLock; nsCOMPtr mIDNWhitelistPrefBranch; - // guarded by mLock bool mIDNUseWhitelist; };