Bug 1487113 - nsHttpChannel should return the Content-Length even when alt-data is available if not delivered, r=valentin

Without this patch, content-length getter returns 0 if we have an available
alt-data because we don't distinguish between having and using it.

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

--HG--
extra : moz-landing-system : lando

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -166,7 +166,7 @@ HttpBaseChannel::HttpBaseChannel()
       mRequestContextID(0),
       mContentWindowId(0),
       mTopLevelOuterContentWindowId(0),
-      mAltDataLength(0),
+      mAltDataLength(-1),
       mChannelId(0),
       mReqContentLength(0U),
       mStatus(NS_OK),
@@ -221,6 +221,7 @@ HttpBaseChannel::HttpBaseChannel()
       mInternalRedirectCount(0),
       mAsyncOpenTimeOverriden(false),
       mForcePending(false),
+      mDeliveringAltData(false),
       mCorsIncludeCredentials(false),
       mOnStartRequestCalled(false),
       mOnStopRequestCalled(false),
@@ -729,7 +730,8 @@ HttpBaseChannel::GetContentLength(int64_t* aContentLength) {
 
   if (!mResponseHead) return NS_ERROR_NOT_AVAILABLE;
 
-  if (!mAvailableCachedAltDataType.IsEmpty()) {
+  if (mDeliveringAltData) {
+    MOZ_ASSERT(!mAvailableCachedAltDataType.IsEmpty());
     *aContentLength = mAltDataLength;
     return NS_OK;
   }
@@ -1192,7 +1194,8 @@ HttpBaseChannel::DoApplyContentConversions(nsIStreamListener* aNextListener,
     return NS_OK;
   }
 
-  if (!mAvailableCachedAltDataType.IsEmpty()) {
+  if (mDeliveringAltData) {
+    MOZ_ASSERT(!mAvailableCachedAltDataType.IsEmpty());
     LOG(("not applying conversion because delivering alt-data\n"));
     return NS_OK;
   }

netwerk/protocol/http/HttpBaseChannel.h

@@ -403,6 +403,8 @@ class HttpBaseChannel : public nsHashPropertyBag,
   int64_t GetAltDataLength() { return mAltDataLength; }
   bool IsNavigation();
 
+  bool IsDeliveringAltData() const { return mDeliveringAltData; }
+
   static bool IsReferrerSchemeAllowed(nsIURI *aReferrer);
 
   static void PropagateReferenceIfNeeded(nsIURI *aURI,
@@ -768,6 +770,9 @@ class HttpBaseChannel : public nsHashPropertyBag,
   bool mAsyncOpenTimeOverriden;
   bool mForcePending;
 
+  // true if the channel is deliving alt-data.
+  bool mDeliveringAltData;
+
   bool mCorsIncludeCredentials;
 
   // These parameters are used to ensure that we do not call OnStartRequest and

netwerk/protocol/http/HttpChannelChild.cpp

@@ -405,7 +405,8 @@ class StartRequestEvent : public NeckoTargetChannelEvent<HttpChannelChild> {
       const nsCString& aSecurityInfoSerialization, const NetAddr& aSelfAddr,
       const NetAddr& aPeerAddr, const uint32_t& aCacheKey,
       const nsCString& altDataType, const int64_t& altDataLen,
-      const bool& aApplyConversion, const ResourceTimingStruct& aTiming)
+      const bool& deliveringAltData, const bool& aApplyConversion,
+      const ResourceTimingStruct& aTiming)
       : NeckoTargetChannelEvent<HttpChannelChild>(aChild),
         mChannelStatus(aChannelStatus),
         mResponseHead(aResponseHead),
@@ -424,6 +425,7 @@ class StartRequestEvent : public NeckoTargetChannelEvent<HttpChannelChild> {
         mCacheKey(aCacheKey),
         mAltDataType(altDataType),
         mAltDataLen(altDataLen),
+        mDeliveringAltData(deliveringAltData),
         mLoadInfoForwarder(loadInfoForwarder),
         mTiming(aTiming) {}
 
@@ -434,7 +436,8 @@ class StartRequestEvent : public NeckoTargetChannelEvent<HttpChannelChild> {
         mLoadInfoForwarder, mIsFromCache, mCacheEntryAvailable, mCacheEntryId,
         mCacheFetchCount, mCacheExpirationTime, mCachedCharset,
         mSecurityInfoSerialization, mSelfAddr, mPeerAddr, mCacheKey,
-        mAltDataType, mAltDataLen, mApplyConversion, mTiming);
+        mAltDataType, mAltDataLen, mDeliveringAltData, mApplyConversion,
+        mTiming);
   }
 
  private:
@@ -455,6 +458,7 @@ class StartRequestEvent : public NeckoTargetChannelEvent<HttpChannelChild> {
   uint32_t mCacheKey;
   nsCString mAltDataType;
   int64_t mAltDataLen;
+  bool mDeliveringAltData;
   ParentLoadInfoForwarderArgs mLoadInfoForwarder;
   ResourceTimingStruct mTiming;
 };
@@ -469,8 +473,8 @@ mozilla::ipc::IPCResult HttpChannelChild::RecvOnStartRequest(
     const nsCString& securityInfoSerialization, const NetAddr& selfAddr,
     const NetAddr& peerAddr, const int16_t& redirectCount,
     const uint32_t& cacheKey, const nsCString& altDataType,
-    const int64_t& altDataLen, const bool& aApplyConversion,
-    const ResourceTimingStruct& aTiming) {
+    const int64_t& altDataLen, const bool& deliveringAltData,
+    const bool& aApplyConversion, const ResourceTimingStruct& aTiming) {
   AUTO_PROFILER_LABEL("HttpChannelChild::RecvOnStartRequest", NETWORK);
   LOG(("HttpChannelChild::RecvOnStartRequest [this=%p]\n", this));
   // mFlushedForDiversion and mDivertingToParent should NEVER be set at this
@@ -489,7 +493,7 @@ mozilla::ipc::IPCResult HttpChannelChild::RecvOnStartRequest(
       loadInfoForwarder, isFromCache, cacheEntryAvailable, cacheEntryId,
       cacheFetchCount, cacheExpirationTime, cachedCharset,
       securityInfoSerialization, selfAddr, peerAddr, cacheKey, altDataType,
-      altDataLen, aApplyConversion, aTiming));
+      altDataLen, deliveringAltData, aApplyConversion, aTiming));
 
   {
     // Child's mEventQ is to control the execution order of the IPC messages
@@ -522,7 +526,8 @@ void HttpChannelChild::OnStartRequest(
     const nsCString& securityInfoSerialization, const NetAddr& selfAddr,
     const NetAddr& peerAddr, const uint32_t& cacheKey,
     const nsCString& altDataType, const int64_t& altDataLen,
-    const bool& aApplyConversion, const ResourceTimingStruct& aTiming) {
+    const bool& deliveringAltData, const bool& aApplyConversion,
+    const ResourceTimingStruct& aTiming) {
   LOG(("HttpChannelChild::OnStartRequest [this=%p]\n", this));
 
   // mFlushedForDiversion and mDivertingToParent should NEVER be set at this
@@ -573,6 +578,7 @@ void HttpChannelChild::OnStartRequest(
   mPeerAddr = peerAddr;
 
   mAvailableCachedAltDataType = altDataType;
+  mDeliveringAltData = deliveringAltData;
   mAltDataLength = altDataLen;
 
   SetApplyConversion(aApplyConversion);

netwerk/protocol/http/HttpChannelChild.h

@@ -144,7 +144,8 @@ class HttpChannelChild final : public PHttpChannelChild,
       const nsCString& securityInfoSerialization, const NetAddr& selfAddr,
       const NetAddr& peerAddr, const int16_t& redirectCount,
       const uint32_t& cacheKey, const nsCString& altDataType,
-      const int64_t& altDataLen, const bool& aApplyConversion,
+      const int64_t& altDataLen, const bool& deliveringAltData,
+      const bool& aApplyConversion,
       const ResourceTimingStruct& aTiming) override;
   mozilla::ipc::IPCResult RecvFailedAsyncOpen(const nsresult& status) override;
   mozilla::ipc::IPCResult RecvRedirect1Begin(
@@ -468,7 +469,8 @@ class HttpChannelChild final : public PHttpChannelChild,
       const nsCString& securityInfoSerialization, const NetAddr& selfAddr,
       const NetAddr& peerAddr, const uint32_t& cacheKey,
       const nsCString& altDataType, const int64_t& altDataLen,
-      const bool& aApplyConversion, const ResourceTimingStruct& aTiming);
+      const bool& deliveringAltData, const bool& aApplyConversion,
+      const ResourceTimingStruct& aTiming);
   void MaybeDivertOnData(const nsCString& data, const uint64_t& offset,
                          const uint32_t& count);
   void OnTransportAndData(const nsresult& channelStatus, const nsresult& status,

netwerk/protocol/http/HttpChannelParent.cpp

@@ -1411,6 +1411,7 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
   chan->GetRedirectCount(&redirectCount);
 
   int64_t altDataLen = chan->GetAltDataLength();
+  bool deliveringAltData = chan->IsDeliveringAltData();
 
   nsCOMPtr<nsILoadInfo> loadInfo;
   Unused << chan->GetLoadInfo(getter_AddRefs(loadInfo));
@@ -1455,8 +1456,8 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
           loadInfoForwarderArg, isFromCache, mCacheEntry ? true : false,
           cacheEntryId, fetchCount, expirationTime, cachedCharset,
           secInfoSerialization, chan->GetSelfAddr(), chan->GetPeerAddr(),
-          redirectCount, cacheKey, altDataType, altDataLen, applyConversion,
-          timing)) {
+          redirectCount, cacheKey, altDataType, altDataLen, deliveringAltData,
+          applyConversion, timing)) {
     rv = NS_ERROR_UNEXPECTED;
   }
   requestHead->Exit();

netwerk/protocol/http/PHttpChannel.ipdl

@@ -124,6 +124,7 @@ child:
                        uint32_t            cacheKey,
                        nsCString           altDataType,
                        int64_t             altDataLength,
+                       bool                deliveringAltData,
                        bool                applyConversion,
                        ResourceTimingStruct timing);
 

netwerk/protocol/http/nsHttpChannel.cpp

@@ -1090,10 +1090,9 @@ nsresult nsHttpChannel::SetupTransaction() {
     }
 
     if (mIgnoreCacheEntry) {
-      if (!mAvailableCachedAltDataType.IsEmpty()) {
-        mAvailableCachedAltDataType.Truncate();
-        mAltDataLength = 0;
-      }
+      mAvailableCachedAltDataType.Truncate();
+      mDeliveringAltData = false;
+      mAltDataLength = -1;
       mCacheInputStream.CloseAndRelease();
     }
   }
@@ -4969,12 +4968,14 @@ nsresult nsHttpChannel::OpenCacheInputStream(nsICacheEntry *cacheEntry,
       LOG(("Opened alt-data input stream type=%s", altDataType.get()));
       // We have succeeded.
       mAvailableCachedAltDataType = altDataType;
+      mDeliveringAltData = deliverAltData;
+
+      // Set the correct data size on the channel.
+      Unused << cacheEntry->GetAltDataSize(&altDataSize);
+      mAltDataLength = altDataSize;
 
       if (deliverAltData) {
-        // Set the correct data size on the channel.
-        Unused << cacheEntry->GetAltDataSize(&altDataSize);
         stream = altData;
-        mAltDataLength = altDataSize;
       }
     }
   }
@@ -5296,6 +5297,7 @@ nsresult nsHttpChannel::InitCacheEntry() {
          "recreating cache entry\n"));
     // clean the altData cache and reset this to avoid wrong content length
     mAvailableCachedAltDataType.Truncate();
+    mDeliveringAltData = false;
 
     nsCOMPtr<nsICacheEntry> currentEntry;
     currentEntry.swap(mCacheEntry);
@@ -7478,6 +7480,7 @@ nsHttpChannel::OnStartRequest(nsIRequest *request) {
         }
       }
       mAvailableCachedAltDataType.Truncate();
+      mDeliveringAltData = false;
     } else if (WRONG_RACING_RESPONSE_SOURCE(request)) {
       LOG(("  Early return when racing. This response not needed."));
       return NS_OK;