From 63e600eaf7ddec4166608732b7fed224908faed4 Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Tue, 6 Jan 2015 17:41:15 -0500 Subject: [PATCH] Backed out changeset 577febac3790 (bug 1100024) for Talos xperf regressions. CLOSED TREE --- netwerk/base/src/nsBaseChannel.cpp | 7 +- netwerk/base/src/nsChannelClassifier.cpp | 78 ++++++----------- netwerk/base/src/nsChannelClassifier.h | 17 +--- netwerk/protocol/http/HttpBaseChannel.cpp | 9 -- netwerk/protocol/http/HttpBaseChannel.h | 1 - netwerk/protocol/http/nsHttpChannel.cpp | 86 ++++++------------- netwerk/protocol/http/nsHttpChannel.h | 4 - .../protocol/http/nsIHttpChannelInternal.idl | 6 -- 8 files changed, 64 insertions(+), 144 deletions(-) diff --git a/netwerk/base/src/nsBaseChannel.cpp b/netwerk/base/src/nsBaseChannel.cpp index 7d0b81f45645..08bf49d0884b 100644 --- a/netwerk/base/src/nsBaseChannel.cpp +++ b/netwerk/base/src/nsBaseChannel.cpp @@ -289,10 +289,15 @@ nsBaseChannel::ClassifyURI() return; } + nsresult rv; + if (mLoadFlags & LOAD_CLASSIFY_URI) { nsRefPtr classifier = new nsChannelClassifier(); if (classifier) { - classifier->Start(this); + rv = classifier->Start(this); + if (NS_FAILED(rv)) { + Cancel(rv); + } } else { Cancel(NS_ERROR_OUT_OF_MEMORY); } diff --git a/netwerk/base/src/nsChannelClassifier.cpp b/netwerk/base/src/nsChannelClassifier.cpp index 46625040814f..2e02e39e6d7e 100644 --- a/netwerk/base/src/nsChannelClassifier.cpp +++ b/netwerk/base/src/nsChannelClassifier.cpp @@ -49,8 +49,7 @@ NS_IMPL_ISUPPORTS(nsChannelClassifier, nsIURIClassifierCallback) nsChannelClassifier::nsChannelClassifier() - : mIsAllowListed(false), - mSuspendedChannel(false) + : mIsAllowListed(false) { #if defined(PR_LOGGING) if (!gChannelClassifierLog) @@ -210,20 +209,8 @@ nsChannelClassifier::NotifyTrackingProtectionDisabled(nsIChannel *aChannel) return NS_OK; } -void -nsChannelClassifier::Start(nsIChannel *aChannel) -{ - mChannel = aChannel; - nsresult rv = StartInternal(aChannel); - if (NS_FAILED(rv)) { - // If we aren't getting a callback for any reason, assume a good verdict and - // make sure we resume the channel if necessary. - OnClassifyComplete(NS_OK); - } -} - nsresult -nsChannelClassifier::StartInternal(nsIChannel *aChannel) +nsChannelClassifier::Start(nsIChannel *aChannel) { // Should only be called in the parent process. MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default); @@ -233,12 +220,12 @@ nsChannelClassifier::StartInternal(nsIChannel *aChannel) nsresult status; aChannel->GetStatus(&status); if (NS_FAILED(status)) - return status; + return NS_OK; // Don't bother to run the classifier on a cached load that was - // previously classified as good. + // previously classified. if (HasBeenClassified(aChannel)) { - return NS_ERROR_UNEXPECTED; + return NS_OK; } nsCOMPtr uri; @@ -251,32 +238,32 @@ nsChannelClassifier::StartInternal(nsIChannel *aChannel) nsIProtocolHandler::URI_DANGEROUS_TO_LOAD, &hasFlags); NS_ENSURE_SUCCESS(rv, rv); - if (hasFlags) return NS_ERROR_UNEXPECTED; + if (hasFlags) return NS_OK; rv = NS_URIChainHasFlags(uri, nsIProtocolHandler::URI_IS_LOCAL_FILE, &hasFlags); NS_ENSURE_SUCCESS(rv, rv); - if (hasFlags) return NS_ERROR_UNEXPECTED; + if (hasFlags) return NS_OK; rv = NS_URIChainHasFlags(uri, nsIProtocolHandler::URI_IS_UI_RESOURCE, &hasFlags); NS_ENSURE_SUCCESS(rv, rv); - if (hasFlags) return NS_ERROR_UNEXPECTED; + if (hasFlags) return NS_OK; rv = NS_URIChainHasFlags(uri, nsIProtocolHandler::URI_IS_LOCAL_RESOURCE, &hasFlags); NS_ENSURE_SUCCESS(rv, rv); - if (hasFlags) return NS_ERROR_UNEXPECTED; + if (hasFlags) return NS_OK; nsCOMPtr uriClassifier = do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID, &rv); if (rv == NS_ERROR_FACTORY_NOT_REGISTERED || rv == NS_ERROR_NOT_AVAILABLE) { // no URI classifier, ignore this failure. - return NS_ERROR_NOT_AVAILABLE; + return NS_OK; } NS_ENSURE_SUCCESS(rv, rv); @@ -295,9 +282,7 @@ nsChannelClassifier::StartInternal(nsIChannel *aChannel) rv = uriClassifier->Classify(principal, trackingProtectionEnabled, this, &expectCallback); - if (NS_FAILED(rv)) { - return rv; - } + if (NS_FAILED(rv)) return rv; if (expectCallback) { // Suspend the channel, it will be resumed when we get the classifier @@ -307,16 +292,14 @@ nsChannelClassifier::StartInternal(nsIChannel *aChannel) // Some channels (including nsJSChannel) fail on Suspend. This // shouldn't be fatal, but will prevent malware from being // blocked on these channels. - LOG(("nsChannelClassifier[%p]: Couldn't suspend channel", this)); - return rv; + return NS_OK; } - mSuspendedChannel = true; + mSuspendedChannel = aChannel; +#ifdef DEBUG LOG(("nsChannelClassifier[%p]: suspended channel %p", - this, mChannel.get())); - } else { - LOG(("nsChannelClassifier[%p]: not expecting callback", this)); - return NS_ERROR_FAILURE; + this, mSuspendedChannel.get())); +#endif } return NS_OK; @@ -335,7 +318,8 @@ nsChannelClassifier::MarkEntryClassified(nsresult status) return; } - nsCOMPtr cachingChannel = do_QueryInterface(mChannel); + nsCOMPtr cachingChannel = + do_QueryInterface(mSuspendedChannel); if (!cachingChannel) { return; } @@ -460,18 +444,17 @@ nsChannelClassifier::OnClassifyComplete(nsresult aErrorCode) // Should only be called in the parent process. MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default); - LOG(("nsChannelClassifier[%p]:OnClassifyComplete", this)); if (mSuspendedChannel) { MarkEntryClassified(aErrorCode); if (NS_FAILED(aErrorCode)) { #ifdef DEBUG nsCOMPtr uri; - mChannel->GetURI(getter_AddRefs(uri)); + mSuspendedChannel->GetURI(getter_AddRefs(uri)); nsCString spec; uri->GetSpec(spec); LOG(("nsChannelClassifier[%p]: cancelling channel %p for %s " - "with error code: %x", this, mChannel.get(), + "with error code: %x", this, mSuspendedChannel.get(), spec.get(), aErrorCode)); #endif @@ -479,23 +462,18 @@ nsChannelClassifier::OnClassifyComplete(nsresult aErrorCode) // Do update the security state of the document and fire a security // change event. if (aErrorCode == NS_ERROR_TRACKING_URI) { - SetBlockedTrackingContent(mChannel); + SetBlockedTrackingContent(mSuspendedChannel); } - mChannel->Cancel(aErrorCode); - } + mSuspendedChannel->Cancel(aErrorCode); + } +#ifdef DEBUG LOG(("nsChannelClassifier[%p]: resuming channel %p from " - "OnClassifyComplete", this, mChannel.get())); - mChannel->Resume(); + "OnClassifyComplete", this, mSuspendedChannel.get())); +#endif + mSuspendedChannel->Resume(); + mSuspendedChannel = nullptr; } - nsresult rv; - nsCOMPtr channel = do_QueryInterface(mChannel, &rv); - // Even if we have cancelled the channel, we need to call - // ContinueBeginConnect so that we abort appropriately. - if (NS_SUCCEEDED(rv)) { - channel->ContinueBeginConnect(); - } - mChannel = nullptr; return NS_OK; } diff --git a/netwerk/base/src/nsChannelClassifier.h b/netwerk/base/src/nsChannelClassifier.h index 40e9bbcb78a2..f260344c671c 100644 --- a/netwerk/base/src/nsChannelClassifier.h +++ b/netwerk/base/src/nsChannelClassifier.h @@ -20,27 +20,16 @@ public: NS_DECL_ISUPPORTS NS_DECL_NSIURICLASSIFIERCALLBACK - // Calls nsIURIClassifier.Classify with the principal of the given channel, - // and cancels the channel on a bad verdict. If aChannel is - // nsIHttpChannelInternal, nsChannelClassifier must call - // ContinueBeginConnect once Start has successfully returned. - void Start(nsIChannel *aChannel); + nsresult Start(nsIChannel *aChannel); private: - // True if the channel is on the allow list. + nsCOMPtr mSuspendedChannel; + // Set true if the channel is on the allow list. bool mIsAllowListed; - // True if the channel has been suspended. - bool mSuspendedChannel; - nsCOMPtr mChannel; ~nsChannelClassifier() {} - // Caches good classifications for the channel principal. void MarkEntryClassified(nsresult status); bool HasBeenClassified(nsIChannel *aChannel); - // Helper function so that we ensure we call ContinueBeginConnect once - // Start is called. Returns NS_OK if and only if we will get a callback - // from the classifier service. - nsresult StartInternal(nsIChannel *aChannel); // Whether or not tracking protection should be enabled on this channel. nsresult ShouldEnableTrackingProtection(nsIChannel *aChannel, bool *result); diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp index b98fac3f730f..bd8eebd2ac85 100644 --- a/netwerk/protocol/http/HttpBaseChannel.cpp +++ b/netwerk/protocol/http/HttpBaseChannel.cpp @@ -1401,15 +1401,6 @@ HttpBaseChannel::RedirectTo(nsIURI *newURI) // HttpBaseChannel::nsIHttpChannelInternal //----------------------------------------------------------------------------- -NS_IMETHODIMP -HttpBaseChannel::ContinueBeginConnect() -{ - MOZ_ASSERT(XRE_GetProcessType() != GeckoProcessType_Default, - "The parent overrides this"); - MOZ_ASSERT(false, "This method must be overridden"); - return NS_ERROR_NOT_IMPLEMENTED; -} - NS_IMETHODIMP HttpBaseChannel::GetTopWindowURI(nsIURI **aTopWindowURI) { diff --git a/netwerk/protocol/http/HttpBaseChannel.h b/netwerk/protocol/http/HttpBaseChannel.h index 45cb373168dd..0f5e9dad1a0d 100644 --- a/netwerk/protocol/http/HttpBaseChannel.h +++ b/netwerk/protocol/http/HttpBaseChannel.h @@ -188,7 +188,6 @@ public: NS_IMETHOD GetLastModifiedTime(PRTime* lastModifiedTime) MOZ_OVERRIDE; NS_IMETHOD ForceNoIntercept() MOZ_OVERRIDE; NS_IMETHOD GetTopWindowURI(nsIURI **aTopWindowURI) MOZ_OVERRIDE; - NS_IMETHOD ContinueBeginConnect(); inline void CleanRedirectCacheChainIfNecessary() { diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 290c11ab74f0..212a07c56448 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -22,7 +22,6 @@ #include "nsISeekableStream.h" #include "nsILoadGroupChild.h" #include "nsIProtocolProxyService2.h" -#include "nsIURIClassifier.h" #include "nsMimeTypes.h" #include "nsNetUtil.h" #include "prprf.h" @@ -40,7 +39,6 @@ #include "GeckoProfiler.h" #include "nsIConsoleService.h" #include "mozilla/Attributes.h" -#include "mozilla/Preferences.h" #include "mozilla/VisualEventTracer.h" #include "nsISSLSocketControl.h" #include "sslt.h" @@ -242,7 +240,6 @@ nsHttpChannel::nsHttpChannel() , mIsPartialRequest(0) , mHasAutoRedirectVetoNotifier(0) , mPushedStream(nullptr) - , mLocalBlocklist(false) , mDidReval(false) { LOG(("Creating nsHttpChannel [this=%p]\n", this)); @@ -329,7 +326,7 @@ nsHttpChannel::Connect() mConnectionInfo->SetPrivate(mPrivateBrowsing); mConnectionInfo->SetNoSpdy(mCaps & NS_HTTP_DISALLOW_SPDY); - // Consider opening a TCP connection right away. + // Consider opening a TCP connection right away SpeculativeConnect(); // Don't allow resuming when cache must be used @@ -436,11 +433,11 @@ nsHttpChannel::SpeculativeConnect() // Before we take the latency hit of dealing with the cache, try and // get the TCP (and SSL) handshakes going so they can overlap. - // don't speculate if we are on a local blocklist, on uses of the offline - // application cache, if we are offline, when doing http upgrade (i.e. - // websockets bootstrap), or if we can't do keep-alive (because then we - // couldn't reuse the speculative connection anyhow). - if (mLocalBlocklist || mApplicationCache || gIOService->IsOffline() || + // don't speculate on uses of the offline application cache, + // if we are offline, when doing http upgrade (i.e. websockets bootstrap), + // or if we can't do keep-alive (because then we couldn't reuse + // the speculative connection anyhow). + if (mApplicationCache || gIOService->IsOffline() || mUpgradeProtocolCallback || !(mCaps & NS_HTTP_ALLOW_KEEPALIVE)) return; @@ -4876,21 +4873,6 @@ nsHttpChannel::BeginConnect() if (mAPIRedirectToURI) { return AsyncCall(&nsHttpChannel::HandleAsyncAPIRedirect); } - // Check to see if this principal exists on local blocklists. - if (mLoadFlags & LOAD_CLASSIFY_URI) { - nsCOMPtr classifier = do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID); - if (classifier) { - nsCOMPtr principal = GetPrincipal(false); - bool tp = Preferences::GetBool("privacy.trackingprotection.enabled", - false); - nsresult response = NS_OK; - classifier->ClassifyLocal(principal, tp, &response); - if (NS_FAILED(response)) { - LOG(("nsHttpChannel::Found principal on local blocklist [this=%p]", this)); - mLocalBlocklist = true; - } - } - } // If mTimingEnabled flag is not set after OnModifyRequest() then // clear the already recorded AsyncOpen value for consistency. @@ -4910,7 +4892,7 @@ nsHttpChannel::BeginConnect() if (mLoadFlags & VALIDATE_ALWAYS || BYPASS_LOCAL_CACHE(mLoadFlags)) mCaps |= NS_HTTP_REFRESH_DNS; - if (!mLocalBlocklist && !mConnectionInfo->UsingHttpProxy() && + if (!mConnectionInfo->UsingHttpProxy() && !(mLoadFlags & (LOAD_NO_NETWORK_IO | LOAD_ONLY_FROM_CACHE))) { // Start a DNS lookup very early in case the real open is queued the DNS can // happen in parallel. Do not do so in the presence of an HTTP proxy as @@ -4954,17 +4936,27 @@ nsHttpChannel::BeginConnect() } mCaps &= ~NS_HTTP_ALLOW_PIPELINING; } - if (mCanceled || !mLocalBlocklist) { - return ContinueBeginConnect(); + + // We may have been cancelled already, either by on-modify-request + // listeners or by load group observers; in that case, we should + // not send the request to the server + if (mCanceled) + rv = mStatus; + else + rv = Connect(); + if (NS_FAILED(rv)) { + LOG(("Calling AsyncAbort [rv=%x mCanceled=%i]\n", rv, mCanceled)); + CloseCacheEntry(true); + AsyncAbort(rv); + } else if (mLoadFlags & LOAD_CLASSIFY_URI) { + nsRefPtr classifier = new nsChannelClassifier(); + rv = classifier->Start(this); + if (NS_FAILED(rv)) { + Cancel(rv); + return rv; + } } - MOZ_ASSERT(!mCanceled && mLocalBlocklist); - // nsChannelClassifier must call ContinueBeginConnect after optionally - // cancelling the channel once we have a remote verdict. We call a concrete - // class instead of an nsI* that might be overridden. - nsRefPtr classifier = new nsChannelClassifier(); - LOG(("nsHttpChannel::Starting nsChannelClassifier %p [this=%p]", - classifier.get(), this)); - classifier->Start(this); + return NS_OK; } @@ -5001,30 +4993,6 @@ nsHttpChannel::SetPriority(int32_t value) return NS_OK; } -//----------------------------------------------------------------------------- -// nsHttpChannel::nsIHttpChannelInternal -//----------------------------------------------------------------------------- -NS_IMETHODIMP -nsHttpChannel::ContinueBeginConnect() -{ - LOG(("nsHttpChannel::ContinueBeginConnect [this=%p]", this)); - nsresult rv; - // We may have been cancelled already, either by on-modify-request - // listeners or load group observers or nsChannelClassifier; in that case, - // we should not send the request to the server - if (mCanceled) { - rv = mStatus; - } else { - rv = Connect(); - } - if (NS_FAILED(rv)) { - LOG(("Calling AsyncAbort [rv=%x mCanceled=%i]\n", rv, mCanceled)); - CloseCacheEntry(true); - AsyncAbort(rv); - } - return rv; -} - //----------------------------------------------------------------------------- // HttpChannel::nsIClassOfService //----------------------------------------------------------------------------- diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h index 23d2f9c3fccb..d7038dcb871d 100644 --- a/netwerk/protocol/http/nsHttpChannel.h +++ b/netwerk/protocol/http/nsHttpChannel.h @@ -119,7 +119,6 @@ public: NS_IMETHOD AsyncOpen(nsIStreamListener *listener, nsISupports *aContext) MOZ_OVERRIDE; // nsIHttpChannelInternal NS_IMETHOD SetupFallbackChannel(const char *aFallbackKey) MOZ_OVERRIDE; - NS_IMETHOD ContinueBeginConnect(); // nsISupportsPriority NS_IMETHOD SetPriority(int32_t value) MOZ_OVERRIDE; // nsIClassOfService @@ -466,9 +465,6 @@ private: nsRefPtr mDNSPrefetch; Http2PushedStream *mPushedStream; - // True if the channel's principal was found on a phishing, malware, or - // tracking (if tracking protection is enabled) blocklist - bool mLocalBlocklist; nsresult WaitForRedirectCallback(); void PushRedirectAsyncFunc(nsContinueRedirectionFunc func); diff --git a/netwerk/protocol/http/nsIHttpChannelInternal.idl b/netwerk/protocol/http/nsIHttpChannelInternal.idl index f3cae28aa7bf..f7df197d53cb 100644 --- a/netwerk/protocol/http/nsIHttpChannelInternal.idl +++ b/netwerk/protocol/http/nsIHttpChannelInternal.idl @@ -224,10 +224,4 @@ interface nsIHttpChannelInternal : nsISupports * The URI of the top-level window that's associated with this channel. */ readonly attribute nsIURI topWindowURI; - - /** - * Used only by nsChannelClassifier to resume connecting or abort the - * channel after a remote classification verdict. - */ - void continueBeginConnect(); };