Bug 1399300 - Backed out changeset a51cf9c048a1 (bug 910207) a=backout
authorValentin Gosu <valentin.gosu@gmail.com>
Wed, 13 Sep 2017 10:51:18 +0200
changeset 430147 de98d45c80054ef0cc675c920c325823b9f9365e
parent 430146 bb26b7c3357a088b54f08c5c9175ee0ea5c2b2a3
child 430148 1ef16717fc89d64d98ef507edf0b8c9820f8cb74
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1399300, 910207
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1399300 - Backed out changeset a51cf9c048a1 (bug 910207) a=backout MozReview-Commit-ID: 3l6B9n7VM1o
netwerk/base/nsISocketTransport.idl
netwerk/base/nsSocketTransport2.cpp
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/protocol/http/nsHttpConnectionInfo.h
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/protocol/http/nsIHttpProtocolHandler.idl
netwerk/socket/nsISSLSocketControl.idl
security/manager/ssl/nsNSSIOLayer.cpp
security/manager/ssl/nsNSSIOLayer.h
--- a/netwerk/base/nsISocketTransport.idl
+++ b/netwerk/base/nsISocketTransport.idl
@@ -233,21 +233,16 @@ interface nsISocketTransport : nsITransp
     /**
      * If set, do not use newer protocol features that might have interop problems
      * on the Internet. Intended only for use with critical infra like the updater.
      * default is false.
      */
     const unsigned long BE_CONSERVATIVE = (1 << 7);
 
     /**
-     * This transport has been created by a speculative connection attempt.
-     */
-    const unsigned long SPECULATIVE = (1 << 8);
-
-    /**
      * An opaque flags for non-standard behavior of the TLS system.
      * It is unlikely this will need to be set outside of telemetry studies
      * relating to the TLS implementation.
      */
     attribute unsigned long tlsFlags;
 
     /**
      * Socket QoS/ToS markings. Valid values are IPTOS_DSCP_AFxx or
--- a/netwerk/base/nsSocketTransport2.cpp
+++ b/netwerk/base/nsSocketTransport2.cpp
@@ -1258,22 +1258,18 @@ nsSocketTransport::BuildSocket(PRFileDes
                 {
                     MutexAutoLock lock(mLock);
                     mSecInfo = secinfo;
                     callbacks = mCallbacks;
                     SOCKET_LOG(("  [secinfo=%p callbacks=%p]\n", mSecInfo.get(), mCallbacks.get()));
                 }
                 // don't call into PSM while holding mLock!!
                 nsCOMPtr<nsISSLSocketControl> secCtrl(do_QueryInterface(secinfo));
-                if (secCtrl) {
-                    if (mConnectionFlags & nsISocketTransport::SPECULATIVE) {
-                        secCtrl->SetSpeculative(true);
-                    }
+                if (secCtrl)
                     secCtrl->SetNotificationCallbacks(callbacks);
-                }
                 // remember if socket type is SSL so we can ProxyStartSSL if need be.
                 usingSSL = isSSL;
             }
             else if ((strcmp(mTypes[i], "socks") == 0) ||
                      (strcmp(mTypes[i], "socks4") == 0)) {
                 // since socks is transparent, any layers above
                 // it do not have to worry about proxy stuff
                 proxyInfo = nullptr;
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -951,28 +951,16 @@ nsHttpConnection::CanReuse()
         return false;
     }
 
     if ((mTransaction ? (mTransaction->IsDone() ? 0U : 1U) : 0U) >=
         mRemainingConnectionUses) {
         return false;
     }
 
-    if (!mExperienced) {
-        uint32_t flags = 0;
-        mSocketTransport->GetConnectionFlags(&flags);
-        if (flags & nsISocketTransport::SPECULATIVE) {
-            if (gHttpHandler->ConnMgr()->IsSpeculativeConnectDisabled(mConnInfo)) {
-                LOG(("nsHttpConnection::CanReuse %p can't reuse because speculative"
-                      " connections are disabled for this host", this));
-                return false;
-            }
-        }
-    }
-
     bool canReuse;
     if (mSpdySession) {
         canReuse = mSpdySession->CanReuse();
     } else {
         canReuse = IsKeepAlive();
     }
     canReuse = canReuse && (IdleTime() < mIdleTimeout) && IsAlive();
 
--- a/netwerk/protocol/http/nsHttpConnectionInfo.h
+++ b/netwerk/protocol/http/nsHttpConnectionInfo.h
@@ -80,17 +80,16 @@ public:
     void SetNetworkInterfaceId(const nsACString& aNetworkInterfaceId);
 
     // OK to treat these as an infalible allocation
     nsHttpConnectionInfo* Clone() const;
     void CloneAsDirectRoute(nsHttpConnectionInfo **outParam);
     MOZ_MUST_USE nsresult CreateWildCard(nsHttpConnectionInfo **outParam);
 
     const char *ProxyHost() const { return mProxyInfo ? mProxyInfo->Host().get() : nullptr; }
-    const nsCString& GetProxyHost() const { return mProxyInfo ? mProxyInfo->Host() : EmptyCString(); }
     int32_t     ProxyPort() const { return mProxyInfo ? mProxyInfo->Port() : -1; }
     const char *ProxyType() const { return mProxyInfo ? mProxyInfo->Type() : nullptr; }
     const char *ProxyUsername() const { return mProxyInfo ? mProxyInfo->Username().get() : nullptr; }
     const char *ProxyPassword() const { return mProxyInfo ? mProxyInfo->Password().get() : nullptr; }
 
     // Compare this connection info to another...
     // Two connections are 'equal' if they end up talking the same
     // protocol to the same server. This is needed to properly manage
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -3227,42 +3227,16 @@ bool nsHttpConnectionMgr::IsConnEntryUnd
     }
 
     nsTArray<RefPtr<PendingTransactionInfo>> *transactions =
         ent->mPendingTransactionTable.Get(mCurrentTopLevelOuterContentWindowId);
 
     return transactions && !transactions->IsEmpty();
 }
 
-void nsHttpConnectionMgr::DontPreconnect(nsACString const & host, int32_t port)
-{
-    MOZ_ASSERT(OnSocketThread(), "not on socket thread");
-
-    for (auto iter = mCT.Iter(); !iter.Done(); iter.Next()) {
-        RefPtr<nsConnectionEntry> ent = iter.Data();
-        nsHttpConnectionInfo* info = ent->mConnInfo;
-
-        if ((info->GetOrigin() == host && info->OriginPort() == port) ||
-            (info->ProxyInfo() && info->GetProxyHost() == host && info->ProxyPort() == port)) {
-
-            LOG(("nsHttpConnectionMgr::DontPreconnect disabling preconnects on %s",
-                 info->HashKey().get()));
-
-            ent->mDisallowPreconnects = true;
-        }
-    }
-}
-
-bool nsHttpConnectionMgr::IsSpeculativeConnectDisabled(nsHttpConnectionInfo *connInfo)
-{
-    nsConnectionEntry *ent = mCT.GetWeak(connInfo->HashKey());
-    return ent && ent->mDisallowPreconnects;
-}
-
-
 bool nsHttpConnectionMgr::IsThrottleTickerNeeded()
 {
     LOG(("nsHttpConnectionMgr::IsThrottleTickerNeeded"));
 
     if (mActiveTabUnthrottledTransactionsExist &&
         mActiveTransactions[false].Count() > 1) {
         LOG(("  there are unthrottled transactions for both active and bck"));
         return true;
@@ -3696,21 +3670,16 @@ nsHttpConnectionMgr::OnMsgSpeculativeCon
     SpeculativeConnectArgs *args = static_cast<SpeculativeConnectArgs *>(param);
 
     LOG(("nsHttpConnectionMgr::OnMsgSpeculativeConnect [ci=%s]\n",
          args->mTrans->ConnectionInfo()->HashKey().get()));
 
     nsConnectionEntry *ent =
         GetOrCreateConnectionEntry(args->mTrans->ConnectionInfo(), false);
 
-    if (ent->mDisallowPreconnects) {
-        LOG(("  explicitely disabled for this host:port"));
-        return;
-    }
-
     uint32_t parallelSpeculativeConnectLimit =
         gHttpHandler->ParallelSpeculativeConnectLimit();
     bool ignoreIdle = false;
     bool isFromPredictor = false;
     bool allow1918 = false;
 
     if (args->mOverridesOK) {
         parallelSpeculativeConnectLimit = args->mParallelSpeculativeConnectLimit;
@@ -3920,20 +3889,16 @@ nsHalfOpenSocket::SetupStreams(nsISocket
              (isBackup && gHttpHandler->FastFallbackToIPv4())) {
         tmpFlags |= nsISocketTransport::DISABLE_IPV6;
     }
 
     if (!Allow1918()) {
         tmpFlags |= nsISocketTransport::DISABLE_RFC1918;
     }
 
-    if (mSpeculative) {
-        tmpFlags |= nsISocketTransport::SPECULATIVE;
-    }
-
     if (!isBackup && mEnt->mUseFastOpen) {
         socketTransport->SetFastOpenCallback(this);
     }
 
     socketTransport->SetConnectionFlags(tmpFlags);
     socketTransport->SetTlsFlags(ci->GetTlsFlags());
 
     const OriginAttributes& originAttributes = mEnt->mConnInfo->GetOriginAttributes();
@@ -4988,17 +4953,16 @@ ConnectionHandle::TopLevelOuterContentWi
 nsHttpConnectionMgr::
 nsConnectionEntry::nsConnectionEntry(nsHttpConnectionInfo *ci)
     : mConnInfo(ci)
     , mUsingSpdy(false)
     , mPreferIPv4(false)
     , mPreferIPv6(false)
     , mUsedForConnection(false)
     , mDoNotDestroy(false)
-    , mDisallowPreconnects(false)
 {
     MOZ_COUNT_CTOR(nsConnectionEntry);
 
     if (mConnInfo->FirstHopSSL()) {
 #if defined(_WIN64) && defined(WIN95)
         mUseFastOpen = gHttpHandler->UseFastOpen() &&
                        gSocketTransportService->HasFileDesc2PlatformOverlappedIOHandleFunc();
 #else
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -237,25 +237,16 @@ public:
     void TouchThrottlingTimeWindow(bool aEnsureTicker = true);
 
     // return true iff the connection has pending transactions for the active tab.
     // it's mainly used to disallow throttling (stop reading) of a response
     // belonging to the same conn info to free up a connection ASAP.
     // NOTE: relatively expensive to call, there are two hashtable lookups.
     bool IsConnEntryUnderPressure(nsHttpConnectionInfo*);
 
-    // This disables preconnecting for all existing connection entries matching
-    // the host and port.  Existing speculative connections that have never been
-    // used will not be used.
-    void DontPreconnect(nsACString const &host, int32_t port);
-
-    // The information is stored on the entry, not on conn-info (which is just
-    // a passive descriptor).
-    bool IsSpeculativeConnectDisabled(nsHttpConnectionInfo*);
-
     uint64_t CurrentTopLevelOuterContentWindowId()
     {
         return mCurrentTopLevelOuterContentWindowId;
     }
 
 private:
     virtual ~nsHttpConnectionMgr();
 
@@ -323,18 +314,16 @@ private:
         // True if this connection entry has initiated a socket
         bool mUsedForConnection : 1;
 
         // Try using TCP Fast Open.
         bool mUseFastOpen : 1;
 
         bool mDoNotDestroy : 1;
 
-        bool mDisallowPreconnects : 1;
-
         // Set the IP family preference flags according the connected family
         void RecordIPFamilyPreference(uint16_t family);
         // Resets all flags to their default values
         void ResetIPFamilyPreference();
 
         // Return the count of pending transactions for all window ids.
         size_t PendingQLength() const;
 
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -2270,25 +2270,16 @@ nsHttpHandler::GetOscpu(nsACString &valu
 
 NS_IMETHODIMP
 nsHttpHandler::GetMisc(nsACString &value)
 {
     value = mMisc;
     return NS_OK;
 }
 
-NS_IMETHODIMP
-nsHttpHandler::DontPreconnect(nsACString const &host, int32_t port)
-{
-    if (mConnMgr) {
-        mConnMgr->DontPreconnect(host, port);
-    }
-    return NS_OK;
-};
-
 //-----------------------------------------------------------------------------
 // nsHttpHandler::nsIObserver
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 nsHttpHandler::Observe(nsISupports *subject,
                        const char *topic,
                        const char16_t *data)
--- a/netwerk/protocol/http/nsIHttpProtocolHandler.idl
+++ b/netwerk/protocol/http/nsIHttpProtocolHandler.idl
@@ -42,22 +42,16 @@ interface nsIHttpProtocolHandler : nsIPr
      */
     [must_use] readonly attribute ACString oscpu;
 
     /**
      * Get the application comment misc portion.
      */
     [must_use] readonly attribute ACString misc;
 
-    /**
-     * Blocks preconnection for the given host:port
-     * This is used mainly to prevent client cert selection dialogs
-     * to pop-up too early.
-     */
-    void dontPreconnect(in ACString host, in int32_t port);
 };
 
 %{C++
 // ----------- Categories -----------
 /**
  * At initialization time, the HTTP handler will initialize each service
  * registered under this category:
  */
--- a/netwerk/socket/nsISSLSocketControl.idl
+++ b/netwerk/socket/nsISSLSocketControl.idl
@@ -17,23 +17,16 @@ interface nsIX509Cert;
 
 [scriptable, builtinclass, uuid(418265c8-654e-4fbb-ba62-4eed27de1f03)]
 interface nsISSLSocketControl : nsISupports {
     attribute nsIInterfaceRequestor     notificationCallbacks;
 
     void proxyStartSSL();
     void StartTLS();
 
-    /**
-     * Whether this socket has been created by a speculative connection
-     * attempt.  This will mainly prevent client certificate dialog to
-     * pop-up.
-     */
-    attribute boolean speculative;
-
     /* NPN (Next Protocol Negotiation) is a mechanism for
        negotiating the protocol to be spoken inside the SSL
        tunnel during the SSL handshake. The NPNList is the list
        of offered client side protocols. setNPNList() needs to
        be called before any data is read or written (including the
        handshake to be setup correctly. The server determines the
        priority when multiple matches occur, but if there is no overlap
        the first protocol in the list is used. */
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -24,17 +24,16 @@
 #include "nsArray.h"
 #include "nsArrayUtils.h"
 #include "nsCRT.h"
 #include "nsCharSeparatedTokenizer.h"
 #include "nsClientAuthRemember.h"
 #include "nsContentUtils.h"
 #include "nsIClientAuthDialogs.h"
 #include "nsIConsoleService.h"
-#include "nsIHttpProtocolHandler.h"
 #include "nsIPrefService.h"
 #include "nsISocketProvider.h"
 #include "nsIWebProgressListener.h"
 #include "nsNSSCertHelper.h"
 #include "nsNSSComponent.h"
 #include "nsPrintfCString.h"
 #include "nsServiceManagerUtils.h"
 #include "pkix/pkixtypes.h"
@@ -119,17 +118,16 @@ nsNSSSocketInfo::nsNSSSocketInfo(SharedS
                                  uint32_t providerTlsFlags)
   : mFd(nullptr),
     mCertVerificationState(before_cert_verification),
     mSharedState(aState),
     mForSTARTTLS(false),
     mHandshakePending(true),
     mRememberClientAuthCertificate(false),
     mPreliminaryHandshakeDone(false),
-    mSpeculative(false),
     mNPNCompleted(false),
     mEarlyDataAccepted(false),
     mFalseStartCallbackCalled(false),
     mFalseStarted(false),
     mIsFullHandshake(false),
     mHandshakeCompleted(false),
     mJoined(false),
     mSentClientCert(false),
@@ -332,30 +330,16 @@ nsNSSSocketInfo::SetHandshakeCompleted()
     mHandshakeCompleted = true;
 
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
            ("[%p] nsNSSSocketInfo::SetHandshakeCompleted\n", (void*) mFd));
 
     mIsFullHandshake = false; // reset for next handshake on this connection
 }
 
-NS_IMETHODIMP
-nsNSSSocketInfo::GetSpeculative(bool *aSpeculative)
-{
-  *aSpeculative = mSpeculative;
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsNSSSocketInfo::SetSpeculative(bool aSpeculative)
-{
-  mSpeculative = aSpeculative;
-  return NS_OK;
-}
-
 void
 nsNSSSocketInfo::SetNegotiatedNPN(const char* value, uint32_t length)
 {
   if (!value) {
     mNegotiatedNPN.Truncate();
   } else {
     mNegotiatedNPN.Assign(value, length);
   }
@@ -2187,47 +2171,31 @@ nsNSS_SSLGetClientAuthData(void* arg, PR
   if (!socket || !caNames || !pRetCert || !pRetKey) {
     PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0);
     return SECFailure;
   }
 
   RefPtr<nsNSSSocketInfo> info(
     BitwiseCast<nsNSSSocketInfo*, PRFilePrivate*>(socket->higher->secret));
 
-  if (info->IsSpeculative()) {
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-      ("[%p] Blocking speculative SSL connections that ask for client cert creds\n", socket->higher));
-
-    nsCOMPtr<nsIHttpProtocolHandler> handler(
-      do_GetService(NS_NETWORK_PROTOCOL_CONTRACTID_PREFIX "http"));
-
-    if (handler) {
-      handler->DontPreconnect(info->GetHostName(), info->GetPort());
-
-      // Bail, this socket will not be used anyway.
-      PR_SetError(SSL_ERROR_NO_CERTIFICATE, 0);
-      return SECFailure;
-    }
-  }
-
   UniqueCERTCertificate serverCert(SSL_PeerCertificate(socket));
   if (!serverCert) {
     MOZ_ASSERT_UNREACHABLE(
       "Missing server cert should have been detected during server cert auth.");
     PR_SetError(SSL_ERROR_NO_CERTIFICATE, 0);
     return SECFailure;
   }
 
   if (info->GetJoined()) {
     // We refuse to send a client certificate when there are multiple hostnames
     // joined on this connection, because we only show the user one hostname
     // (mHostName) in the client certificate UI.
 
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-           ("[%p] Not returning client cert due to previous join\n", socket->higher));
+           ("[%p] Not returning client cert due to previous join\n", socket));
     *pRetCert = nullptr;
     *pRetKey = nullptr;
     return SECSuccess;
   }
 
   // XXX: This should be done asynchronously; see bug 696976
   RefPtr<ClientAuthDataRunnable> runnable(
     new ClientAuthDataRunnable(caNames, pRetCert, pRetKey, info, serverCert));
--- a/security/manager/ssl/nsNSSIOLayer.h
+++ b/security/manager/ssl/nsNSSIOLayer.h
@@ -44,18 +44,16 @@ public:
   NS_DECL_NSICLIENTAUTHUSERDECISION
 
   void SetForSTARTTLS(bool aForSTARTTLS);
   bool GetForSTARTTLS();
 
   nsresult GetFileDescPtr(PRFileDesc** aFilePtr);
   nsresult SetFileDescPtr(PRFileDesc* aFilePtr);
 
-  bool IsSpeculative() { return mSpeculative; }
-
   bool IsHandshakePending() const { return mHandshakePending; }
   void SetHandshakeNotPending() { mHandshakePending = false; }
 
   void SetTLSVersionRange(SSLVersionRange range) { mTLSVersionRange = range; }
   SSLVersionRange GetTLSVersionRange() const { return mTLSVersionRange; };
 
   PRStatus CloseSocketAndDestroy(
                 const nsNSSShutDownPreventionLock& proofOfLock);
@@ -181,17 +179,16 @@ private:
   SSLVersionRange mTLSVersionRange;
   bool mHandshakePending;
   bool mRememberClientAuthCertificate;
   bool mPreliminaryHandshakeDone; // after false start items are complete
 
   nsresult ActivateSSL();
 
   nsCString mNegotiatedNPN;
-  bool      mSpeculative;
   bool      mNPNCompleted;
   bool      mEarlyDataAccepted;
   bool      mFalseStartCallbackCalled;
   bool      mFalseStarted;
   bool      mIsFullHandshake;
   bool      mHandshakeCompleted;
   bool      mJoined;
   bool      mSentClientCert;