Bug 1598497 - P1. move canceled attribute to nsIChannel. r=mayhemer

There is no functional change with this commit. The default implementation for GetCanceled() is still to check if the status code is a failure.

However, it can be argued that as you had to call Cancel() on the nsIChannel, having to check the nsIHttpChannelInternal interface to determine if you had been canceled in the past was rather a non obvious path.

It makes more sense to check the nsIChannel interface to determine if it's been canceled already and this allows for finer granularity if needed in the future.

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

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

dom/ipc/BrowserParent.cpp

@@ -3620,6 +3620,7 @@ class FakeChannel final : public nsIChannel,
   NS_IMETHOD IsPending(bool*) NO_IMPL;
   NS_IMETHOD GetStatus(nsresult*) NO_IMPL;
   NS_IMETHOD Cancel(nsresult) NO_IMPL;
+  NS_IMETHOD GetCanceled(bool* aCanceled) NO_IMPL;
   NS_IMETHOD Suspend() NO_IMPL;
   NS_IMETHOD Resume() NO_IMPL;
   NS_IMETHOD GetLoadGroup(nsILoadGroup**) NO_IMPL;
@@ -3696,7 +3697,7 @@ class FakeChannel final : public nsIChannel,
 #undef NO_IMPL
 
  protected:
-  ~FakeChannel() {}
+  ~FakeChannel() = default;
 
   nsCOMPtr<nsIURI> mUri;
   uint64_t mCallbackId;

dom/ipc/RemoteWebProgressRequest.cpp

@@ -202,6 +202,10 @@ NS_IMETHODIMP RemoteWebProgressRequest::Cancel(nsresult aStatus) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
+NS_IMETHODIMP RemoteWebProgressRequest::GetCanceled(bool* aCanceled) {
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 NS_IMETHODIMP RemoteWebProgressRequest::Suspend(void) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }

dom/jsurl/nsJSProtocolHandler.cpp

@@ -484,6 +484,14 @@ nsJSChannel::Cancel(nsresult aStatus) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsJSChannel::GetCanceled(bool* aCanceled) {
+  nsresult status = NS_ERROR_FAILURE;
+  GetStatus(&status);
+  *aCanceled = NS_FAILED(status);
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsJSChannel::Suspend() { return mStreamChannel->Suspend(); }
 

image/decoders/icon/win/nsIconChannel.cpp

@@ -202,6 +202,15 @@ nsIconChannel::GetStatus(nsresult* status) { return mPump->GetStatus(status); }
 NS_IMETHODIMP
 nsIconChannel::Cancel(nsresult status) { 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);
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsIconChannel::Suspend(void) { return mPump->Suspend(); }
 

modules/libjar/nsJARChannel.cpp

@@ -572,6 +572,14 @@ nsJARChannel::Cancel(nsresult status) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsJARChannel::GetCanceled(bool* aCanceled) {
+  nsresult status = NS_ERROR_FAILURE;
+  GetStatus(&status);
+  *aCanceled = NS_FAILED(status);
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsJARChannel::Suspend() {
   ++mPendingEvent.suspendCount;

netwerk/base/nsAsyncRedirectVerifyHelper.cpp

@@ -272,25 +272,12 @@ nsAsyncRedirectVerifyHelper::Run() {
 }
 
 bool nsAsyncRedirectVerifyHelper::IsOldChannelCanceled() {
-  bool canceled;
-  nsCOMPtr<nsIHttpChannelInternal> oldChannelInternal =
-      do_QueryInterface(mOldChan);
-  if (oldChannelInternal) {
-    nsresult rv = oldChannelInternal->GetCanceled(&canceled);
-    if (NS_SUCCEEDED(rv) && canceled) {
-      return true;
-    }
-  } else if (mOldChan) {
-    // For non-HTTP channels check on the status, failure
-    // indicates the channel has probably been canceled.
-    nsresult status = NS_ERROR_FAILURE;
-    mOldChan->GetStatus(&status);
-    if (NS_FAILED(status)) {
-      return true;
-    }
+  if (!mOldChan) {
+    return false;
   }
-
-  return false;
+  bool canceled;
+  nsresult rv = mOldChan->GetCanceled(&canceled);
+  return NS_SUCCEEDED(rv) && canceled;
 }
 
 }  // namespace net

netwerk/base/nsBaseChannel.cpp

@@ -942,6 +942,14 @@ nsBaseChannel::CheckListenerChain() {
   return listener->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);
+  return NS_OK;
+}
+
 void nsBaseChannel::SetupNeckoTarget() {
   mNeckoTarget =
       nsContentUtils::GetEventTargetByLoadInfo(mLoadInfo, TaskCategory::Other);

netwerk/base/nsIChannel.idl

@@ -188,6 +188,11 @@ interface nsIChannel : nsIRequest
      */
     void asyncOpen(in nsIStreamListener aListener);
 
+    /**
+     * True if the channel has been canceled.
+     */
+    [must_use] readonly attribute boolean canceled;
+
     /**************************************************************************
      * Channel specific load flags:
      *

netwerk/ipc/DocumentChannelChild.cpp

@@ -717,6 +717,11 @@ NS_IMETHODIMP DocumentChannelChild::GetIsDocument(bool* aIsDocument) {
   return NS_GetIsDocumentChannel(this, aIsDocument);
 }
 
+NS_IMETHODIMP DocumentChannelChild::GetCanceled(bool* aCanceled) {
+  *aCanceled = mCanceled;
+  return NS_OK;
+}
+
 //-----------------------------------------------------------------------------
 // nsIIdentChannel
 //-----------------------------------------------------------------------------

netwerk/protocol/about/nsAboutCache.h

@@ -101,6 +101,9 @@
   }                                                                            \
   NS_IMETHOD GetIsDocument(bool* aIsDocument) override {                       \
     return !_to ? NS_ERROR_NULL_POINTER : _to->GetIsDocument(aIsDocument);     \
+  }                                                                            \
+  NS_IMETHOD GetCanceled(bool* aCanceled) override {                           \
+    return !_to ? NS_ERROR_NULL_POINTER : _to->GetCanceled(aCanceled);         \
   };
 
 class nsAboutCache final : public nsIAboutModule {

netwerk/protocol/http/HttpBaseChannel.h

@@ -171,6 +171,7 @@ class HttpBaseChannel : public nsHashPropertyBag,
   NS_IMETHOD Open(nsIInputStream** aResult) override;
   NS_IMETHOD GetBlockAuthPrompt(bool* aValue) override;
   NS_IMETHOD SetBlockAuthPrompt(bool aValue) override;
+  NS_IMETHOD GetCanceled(bool* aCanceled) override;
 
   // nsIEncodedChannel
   NS_IMETHOD GetApplyConversion(bool* value) override;
@@ -253,7 +254,6 @@ class HttpBaseChannel : public nsHashPropertyBag,
   NS_IMETHOD SetThirdPartyFlags(uint32_t aForce) override;
   NS_IMETHOD GetForceAllowThirdPartyCookie(bool* aForce) override;
   NS_IMETHOD SetForceAllowThirdPartyCookie(bool aForce) override;
-  NS_IMETHOD GetCanceled(bool* aCanceled) override;
   NS_IMETHOD GetChannelIsForDownload(bool* aChannelIsForDownload) override;
   NS_IMETHOD SetChannelIsForDownload(bool aChannelIsForDownload) override;
   NS_IMETHOD SetCacheKeysRedirectChain(nsTArray<nsCString>* cacheKeys) override;

netwerk/protocol/http/NullHttpChannel.cpp

@@ -428,6 +428,11 @@ NullHttpChannel::GetStatus(nsresult* aStatus) {
 NS_IMETHODIMP
 NullHttpChannel::Cancel(nsresult aStatus) { return NS_ERROR_NOT_IMPLEMENTED; }
 
+NS_IMETHODIMP
+NullHttpChannel::GetCanceled(bool* aCanceled) {
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 NS_IMETHODIMP
 NullHttpChannel::Suspend() { return NS_ERROR_NOT_IMPLEMENTED; }
 

netwerk/protocol/http/nsIHttpChannelInternal.idl

@@ -108,11 +108,6 @@ interface nsIHttpChannelInternal : nsISupports
      */
     [must_use] attribute boolean forceAllowThirdPartyCookie;
 
-    /**
-     * True iff the channel has been canceled.
-     */
-    [must_use] readonly attribute boolean canceled;
-
     /**
      * External handlers may set this to true to notify the channel
      * that it is open on behalf of a download.

netwerk/protocol/viewsource/nsViewSourceChannel.cpp

@@ -235,6 +235,13 @@ nsViewSourceChannel::Cancel(nsresult status) {
   return mChannel->Cancel(status);
 }
 
+NS_IMETHODIMP
+nsViewSourceChannel::GetCanceled(bool* aCanceled) {
+  NS_ENSURE_TRUE(mChannel, NS_ERROR_FAILURE);
+
+  return mChannel->GetCanceled(aCanceled);
+}
+
 NS_IMETHODIMP
 nsViewSourceChannel::Suspend(void) {
   NS_ENSURE_TRUE(mChannel, NS_ERROR_FAILURE);

netwerk/streamconv/converters/nsMultiMixedConv.cpp

@@ -130,6 +130,12 @@ nsPartChannel::Cancel(nsresult aStatus) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsPartChannel::GetCanceled(bool* aCanceled) {
+  *aCanceled = NS_FAILED(mStatus);
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsPartChannel::Suspend(void) {
   // Suspending an individual part must not suspend the underlying

uriloader/exthandler/ExternalHelperAppParent.cpp

@@ -222,6 +222,12 @@ ExternalHelperAppParent::Cancel(nsresult aStatus) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+ExternalHelperAppParent::GetCanceled(bool* aCanceled) {
+  *aCanceled = NS_FAILED(mStatus);
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 ExternalHelperAppParent::Suspend() { return NS_ERROR_NOT_IMPLEMENTED; }
 

uriloader/exthandler/nsExternalProtocolHandler.cpp

@@ -337,6 +337,11 @@ NS_IMETHODIMP nsExtProtocolChannel::Cancel(nsresult status) {
   return NS_OK;
 }
 
+NS_IMETHODIMP nsExtProtocolChannel::GetCanceled(bool* aCanceled) {
+  *aCanceled = NS_FAILED(mStatus);
+  return NS_OK;
+}
+
 NS_IMETHODIMP nsExtProtocolChannel::Suspend() {
   MOZ_ASSERT_UNREACHABLE("Suspend");
   return NS_ERROR_NOT_IMPLEMENTED;