Bug 1598497 - P2. Have the canceled attribute only if explicitly canceled. r=mayhemer.

Previously, GetCancelled() would have return true should the channel's status was an error.
Doing Cancel(NS_OK) for example, would have made a follow-up call to GetCancelled() return false. However, we can assert that such a call would have been a bug.

Following this change GetCancelled() will only return true if Cancel() was explicitly called.

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

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

image/decoders/icon/mac/nsIconChannel.h

@@ -47,6 +47,7 @@ class nsIconChannel final : public nsIChannel, public nsIStreamListener {
 
   nsCOMPtr<nsIInputStreamPump> mPump;
   nsCOMPtr<nsIStreamListener> mListener;
+  bool mCanceled = false;
 
   MOZ_MUST_USE nsresult MakeInputStream(nsIInputStream** _retval,
                                         bool nonBlocking);

image/decoders/icon/mac/nsIconChannelCocoa.mm

@@ -65,7 +65,16 @@ NS_IMETHODIMP
 nsIconChannel::GetStatus(nsresult* status) { return mPump->GetStatus(status); }
 
 NS_IMETHODIMP
-nsIconChannel::Cancel(nsresult status) { return mPump->Cancel(status); }
+nsIconChannel::Cancel(nsresult status) {
+  mCanceled = true;
+  return mPump->Cancel(status);
+}
+
+NS_IMETHODIMP
+nsIconChannel::GetCanceled(bool* result) {
+  *result = mCanceled;
+  return NS_OK;
+}
 
 NS_IMETHODIMP
 nsIconChannel::Suspend(void) { return mPump->Suspend(); }

image/decoders/icon/win/nsIconChannel.cpp

@@ -200,14 +200,14 @@ NS_IMETHODIMP
 nsIconChannel::GetStatus(nsresult* status) { return mPump->GetStatus(status); }
 
 NS_IMETHODIMP
-nsIconChannel::Cancel(nsresult status) { return mPump->Cancel(status); }
+nsIconChannel::Cancel(nsresult status) {
+  mCanceled = true;
+  return mPump->Cancel(status);
+}
 
 NS_IMETHODIMP
 nsIconChannel::GetCanceled(bool* result) {
-  // Failure indicates the channel has probably been canceled.
-  nsresult status = NS_ERROR_FAILURE;
-  GetStatus(&status);
-  *result = NS_FAILED(status);
+  *result = mCanceled;
   return NS_OK;
 }
 

image/decoders/icon/win/nsIconChannel.h

@@ -75,6 +75,9 @@ class nsIconChannel final : public nsIChannel, public nsIStreamListener {
   // Functions specific to Vista and above
  protected:
   nsresult GetStockHIcon(nsIMozIconURI* aIconURI, HICON* hIcon);
+
+ private:
+  bool mCanceled = false;
 };
 
 #endif  // mozilla_image_encoders_icon_win_nsIconChannel_h

modules/libjar/nsJARChannel.cpp

@@ -167,6 +167,7 @@ nsJARInputThunk::IsNonBlocking(bool* nonBlocking) {
 
 nsJARChannel::nsJARChannel()
     : mOpened(false),
+      mCanceled(false),
       mContentLength(-1),
       mLoadFlags(LOAD_NORMAL),
       mStatus(NS_OK),
@@ -560,6 +561,7 @@ nsJARChannel::GetStatus(nsresult* status) {
 
 NS_IMETHODIMP
 nsJARChannel::Cancel(nsresult status) {
+  mCanceled = true;
   mStatus = status;
   if (mPump) {
     return mPump->Cancel(status);
@@ -574,9 +576,7 @@ nsJARChannel::Cancel(nsresult status) {
 
 NS_IMETHODIMP
 nsJARChannel::GetCanceled(bool* aCanceled) {
-  nsresult status = NS_ERROR_FAILURE;
-  GetStatus(&status);
-  *aCanceled = NS_FAILED(status);
+  *aCanceled = mCanceled;
   return NS_OK;
 }
 

modules/libjar/nsJARChannel.h

@@ -68,6 +68,7 @@ class nsJARChannel final : public nsIJARChannel,
   nsCString mSpec;
 
   bool mOpened;
+  bool mCanceled;
 
   RefPtr<nsJARProtocolHandler> mJarHandler;
   nsCOMPtr<nsIJARURI> mJarURI;

netwerk/base/nsBaseChannel.cpp

@@ -64,7 +64,8 @@ nsBaseChannel::nsBaseChannel()
       mStatus(NS_OK),
       mContentDispositionHint(UINT32_MAX),
       mContentLength(-1),
-      mWasOpened(false) {
+      mWasOpened(false),
+      mCanceled(false) {
   mContentType.AssignLiteral(UNKNOWN_CONTENT_TYPE);
 }
 
@@ -391,11 +392,16 @@ nsBaseChannel::GetStatus(nsresult* status) {
 NS_IMETHODIMP
 nsBaseChannel::Cancel(nsresult status) {
   // Ignore redundant cancelation
-  if (NS_FAILED(mStatus)) return NS_OK;
+  if (mCanceled) {
+    return NS_OK;
+  }
 
+  mCanceled = true;
   mStatus = status;
 
-  if (mRequest) mRequest->Cancel(status);
+  if (mRequest) {
+    mRequest->Cancel(status);
+  }
 
   return NS_OK;
 }
@@ -943,10 +949,7 @@ nsBaseChannel::CheckListenerChain() {
 }
 
 NS_IMETHODIMP nsBaseChannel::GetCanceled(bool* aCanceled) {
-  // Failure indicates the channel has probably been canceled.
-  nsresult status = NS_ERROR_FAILURE;
-  GetStatus(&status);
-  *aCanceled = NS_FAILED(status);
+  *aCanceled = mCanceled;
   return NS_OK;
 }
 

netwerk/base/nsBaseChannel.h

@@ -298,6 +298,7 @@ class nsBaseChannel
   nsAutoPtr<nsString> mContentDispositionFilename;
   int64_t mContentLength;
   bool mWasOpened;
+  bool mCanceled;
 
   friend class mozilla::net::PrivateBrowsingChannel<nsBaseChannel>;
 };

uriloader/exthandler/ExternalHelperAppParent.cpp

@@ -48,6 +48,7 @@ ExternalHelperAppParent::ExternalHelperAppParent(
       mIPCClosed(false),
       mLoadFlags(0),
       mStatus(NS_OK),
+      mCanceled(false),
       mContentLength(aContentLength),
       mWasFileChannel(aWasFileChannel) {
   mContentDispositionHeader = aContentDispositionHeader;
@@ -217,6 +218,7 @@ ExternalHelperAppParent::GetStatus(nsresult* aResult) {
 
 NS_IMETHODIMP
 ExternalHelperAppParent::Cancel(nsresult aStatus) {
+  mCanceled = true;
   mStatus = aStatus;
   Unused << SendCancel(aStatus);
   return NS_OK;
@@ -224,7 +226,7 @@ ExternalHelperAppParent::Cancel(nsresult aStatus) {
 
 NS_IMETHODIMP
 ExternalHelperAppParent::GetCanceled(bool* aCanceled) {
-  *aCanceled = NS_FAILED(mStatus);
+  *aCanceled = mCanceled;
   return NS_OK;
 }
 

uriloader/exthandler/ExternalHelperAppParent.h

@@ -112,6 +112,7 @@ class ExternalHelperAppParent
   bool mIPCClosed;
   nsLoadFlags mLoadFlags;
   nsresult mStatus;
+  bool mCanceled;
   int64_t mContentLength;
   bool mWasFileChannel;
   uint32_t mContentDisposition;

uriloader/exthandler/nsExternalProtocolHandler.cpp

@@ -58,6 +58,7 @@ class nsExtProtocolChannel : public nsIChannel,
   nsresult mStatus;
   nsLoadFlags mLoadFlags;
   bool mWasOpened;
+  bool mCanceled;
   // Set true (as a result of ConnectParent invoked from child process)
   // when this channel is on the parent process and is being used as
   // a redirect target channel.  It turns AsyncOpen into a no-op since
@@ -89,6 +90,7 @@ nsExtProtocolChannel::nsExtProtocolChannel(nsIURI* aURI, nsILoadInfo* aLoadInfo)
       mStatus(NS_OK),
       mLoadFlags(nsIRequest::LOAD_NORMAL),
       mWasOpened(false),
+      mCanceled(false),
       mConnectedParent(false),
       mLoadInfo(aLoadInfo) {}
 
@@ -334,11 +336,12 @@ NS_IMETHODIMP nsExtProtocolChannel::Cancel(nsresult status) {
   if (NS_SUCCEEDED(mStatus)) {
     mStatus = status;
   }
+  mCanceled = true;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsExtProtocolChannel::GetCanceled(bool* aCanceled) {
-  *aCanceled = NS_FAILED(mStatus);
+  *aCanceled = mCanceled;
   return NS_OK;
 }