Bug 1487113 - AltDataOutputStreamChild must be nsIAsyncOutputStream, r=valentin

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

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

netwerk/protocol/http/AltDataOutputStreamChild.cpp

@@ -1,6 +1,7 @@
 #include "mozilla/net/AltDataOutputStreamChild.h"
 #include "mozilla/Unused.h"
 #include "nsIInputStream.h"
+#include "nsStreamUtils.h"
 
 namespace mozilla {
 namespace net {
@@ -30,12 +31,13 @@ NS_IMETHODIMP_(MozExternalRefCountType) AltDataOutputStreamChild::Release() {
 }
 
 NS_INTERFACE_MAP_BEGIN(AltDataOutputStreamChild)
+  NS_INTERFACE_MAP_ENTRY(nsIAsyncOutputStream)
   NS_INTERFACE_MAP_ENTRY(nsIOutputStream)
   NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
 
 AltDataOutputStreamChild::AltDataOutputStreamChild()
-    : mIPCOpen(false), mError(NS_OK) {
+    : mIPCOpen(false), mError(NS_OK), mCallbackFlags(0) {
   MOZ_ASSERT(NS_IsMainThread(), "Main thread only");
 }
 
@@ -48,6 +50,11 @@ void AltDataOutputStreamChild::AddIPDLReference() {
 void AltDataOutputStreamChild::ReleaseIPDLReference() {
   MOZ_ASSERT(mIPCOpen, "Attempt to release nonexistent IPDL reference");
   mIPCOpen = false;
+
+  if (mCallback) {
+    NotifyListener();
+  }
+
   Release();
 }
 
@@ -67,16 +74,7 @@ bool AltDataOutputStreamChild::WriteDataInChunks(
 }
 
 NS_IMETHODIMP
-AltDataOutputStreamChild::Close() {
-  if (!mIPCOpen) {
-    return NS_ERROR_NOT_AVAILABLE;
-  }
-  if (NS_FAILED(mError)) {
-    return mError;
-  }
-  Unused << SendClose();
-  return NS_OK;
-}
+AltDataOutputStreamChild::Close() { return CloseWithStatus(NS_OK); }
 
 NS_IMETHODIMP
 AltDataOutputStreamChild::Flush() {
@@ -137,5 +135,56 @@ mozilla::ipc::IPCResult AltDataOutputStreamChild::RecvDeleteSelf() {
   return IPC_OK();
 }
 
+// nsIAsyncOutputStream
+
+NS_IMETHODIMP
+AltDataOutputStreamChild::CloseWithStatus(nsresult aStatus) {
+  if (!mIPCOpen) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+  if (NS_FAILED(mError)) {
+    return mError;
+  }
+  Unused << SendClose(aStatus);
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+AltDataOutputStreamChild::AsyncWait(nsIOutputStreamCallback *aCallback,
+                                    uint32_t aFlags, uint32_t aRequestedCount,
+                                    nsIEventTarget *aEventTarget) {
+  mCallback = aCallback;
+  mCallbackFlags = aFlags;
+  mCallbackTarget = aEventTarget;
+
+  if (!mCallback) {
+    return NS_OK;
+  }
+
+  // The stream is blocking so it is writable at any time
+  if (!mIPCOpen || !(aFlags & WAIT_CLOSURE_ONLY)) {
+    NotifyListener();
+  }
+
+  return NS_OK;
+}
+
+void AltDataOutputStreamChild::NotifyListener() {
+  MOZ_ASSERT(mCallback);
+
+  if (!mCallbackTarget) {
+    mCallbackTarget = GetMainThreadEventTarget();
+  }
+
+  nsCOMPtr<nsIOutputStreamCallback> asyncCallback =
+      NS_NewOutputStreamReadyEvent(mCallback, mCallbackTarget);
+
+  mCallback = nullptr;
+  mCallbackTarget = nullptr;
+
+  asyncCallback->OnOutputStreamReady(this);
+}
+
 }  // namespace net
 }  // namespace mozilla

netwerk/protocol/http/AltDataOutputStreamChild.h

@@ -9,15 +9,16 @@
 #define mozilla_net_AltDataOutputStreamChild_h
 
 #include "mozilla/net/PAltDataOutputStreamChild.h"
-#include "nsIOutputStream.h"
+#include "nsIAsyncOutputStream.h"
 
 namespace mozilla {
 namespace net {
 
 class AltDataOutputStreamChild : public PAltDataOutputStreamChild,
-                                 public nsIOutputStream {
+                                 public nsIAsyncOutputStream {
  public:
   NS_DECL_ISUPPORTS
+  NS_DECL_NSIASYNCOUTPUTSTREAM
   NS_DECL_NSIOUTPUTSTREAM
   explicit AltDataOutputStreamChild();
 
@@ -31,12 +32,17 @@ class AltDataOutputStreamChild : public PAltDataOutputStreamChild,
   virtual ~AltDataOutputStreamChild() = default;
   // Sends data to the parent process in 256k chunks.
   bool WriteDataInChunks(const nsDependentCSubstring& data);
+  void NotifyListener();
 
   bool mIPCOpen;
   // If there was an error opening the output stream or writing to it on the
   // parent side, this will be set to the error code. We check it before we
   // write so we can report an error to the consumer.
   nsresult mError;
+
+  nsCOMPtr<nsIOutputStreamCallback> mCallback;
+  uint32_t mCallbackFlags;
+  nsCOMPtr<nsIEventTarget> mCallbackTarget;
 };
 
 }  // namespace net

netwerk/protocol/http/AltDataOutputStreamParent.cpp

@@ -42,21 +42,29 @@ mozilla::ipc::IPCResult AltDataOutputStreamParent::RecvWriteData(
   return IPC_OK();
 }
 
-mozilla::ipc::IPCResult AltDataOutputStreamParent::RecvClose() {
+mozilla::ipc::IPCResult AltDataOutputStreamParent::RecvClose(
+    const nsresult& aStatus) {
   if (NS_FAILED(mStatus)) {
     if (mIPCOpen) {
       Unused << SendError(mStatus);
     }
     return IPC_OK();
   }
-  nsresult rv;
-  if (mOutputStream) {
-    rv = mOutputStream->Close();
-    if (NS_FAILED(rv) && mIPCOpen) {
-      Unused << SendError(rv);
-    }
-    mOutputStream = nullptr;
+
+  if (!mOutputStream) {
+    return IPC_OK();
   }
+
+  nsCOMPtr<nsIAsyncOutputStream> asyncOutputStream =
+      do_QueryInterface(mOutputStream);
+  MOZ_ASSERT(asyncOutputStream);
+
+  nsresult rv = asyncOutputStream->CloseWithStatus(aStatus);
+  if (NS_FAILED(rv) && mIPCOpen) {
+    Unused << SendError(rv);
+  }
+
+  mOutputStream = nullptr;
   return IPC_OK();
 }
 

netwerk/protocol/http/AltDataOutputStreamParent.h

@@ -30,7 +30,7 @@ class AltDataOutputStreamParent : public PAltDataOutputStreamParent,
   mozilla::ipc::IPCResult RecvWriteData(const nsCString& data);
   // Called when AltDataOutputStreamChild::Close() is
   // Closes and nulls the output stream.
-  mozilla::ipc::IPCResult RecvClose();
+  mozilla::ipc::IPCResult RecvClose(const nsresult& aStatus);
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   // Sets an error that will be reported to the content process.

netwerk/protocol/http/PAltDataOutputStream.ipdl

@@ -18,7 +18,7 @@ parent:
   // Sends data from the child to the parent that will be written to the cache.
   async WriteData(nsCString data);
   // Signals that writing to the output stream is done.
-  async Close();
+  async Close(nsresult status);
 
   async __delete__();
 

netwerk/test/unit/test_alt-data_closeWithStatus.js

@@ -0,0 +1,163 @@
+/**
+ * Test for the "alternative data stream" - closing the stream with an error.
+ *
+ * - we load a URL with preference for an alt data (check what we get is the raw data,
+ *   since there was nothing previously cached)
+ * - we store something in alt data (using the asyncWait method)
+ * - then we abort the operation calling closeWithStatus()
+ * - we flush the HTTP cache
+ * - we reload the same URL using a new channel, again prefering the alt data be loaded
+ * - again we receive the data from the server.
+ */
+
+const {HttpServer} = ChromeUtils.import("resource://testing-common/httpd.js");
+const {NetUtil} = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
+
+XPCOMUtils.defineLazyGetter(this, "URL", function() {
+  return "http://localhost:" + httpServer.identity.primaryPort + "/content";
+});
+
+var httpServer = null;
+
+// needs to be rooted
+var cacheFlushObserver = cacheFlushObserver = { observe: function() {
+  cacheFlushObserver = null;
+  readServerContentAgain();
+}};
+
+var currentThread = null;
+
+function make_channel(url, callback, ctx) {
+  return NetUtil.newChannel({uri: url, loadUsingSystemPrincipal: true});
+}
+
+function inChildProcess() {
+  return Cc["@mozilla.org/xre/app-info;1"]
+           .getService(Ci.nsIXULRuntime)
+           .processType != Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT;
+}
+
+const responseContent = "response body";
+const responseContent2 = "response body 2";
+const altContent = "!@#$%^&*()";
+const altContentType = "text/binary";
+
+var servedNotModified = false;
+var shouldPassRevalidation = true;
+
+var cache_storage = null;
+
+function contentHandler(metadata, response)
+{
+  response.setHeader("Content-Type", "text/plain");
+  response.setHeader("Cache-Control", "no-cache");
+  response.setHeader("ETag", "test-etag1");
+
+  try {
+    var etag = metadata.getHeader("If-None-Match");
+  } catch(ex) {
+    var etag = "";
+  }
+
+  if (etag == "test-etag1" && shouldPassRevalidation) {
+    response.setStatusLine(metadata.httpVersion, 304, "Not Modified");
+    servedNotModified = true;
+  } else {
+    var content = shouldPassRevalidation ? responseContent : responseContent2;
+    response.bodyOutputStream.write(content, content.length);
+  }
+}
+
+function check_has_alt_data_in_index(aHasAltData)
+{
+  if (inChildProcess()) {
+    return;
+  }
+  var hasAltData = {};
+  cache_storage.getCacheIndexEntryAttrs(createURI(URL), "", hasAltData, {});
+  Assert.equal(hasAltData.value, aHasAltData);
+}
+
+function run_test()
+{
+  do_get_profile();
+  httpServer = new HttpServer();
+  httpServer.registerPathHandler("/content", contentHandler);
+  httpServer.start(-1);
+  do_test_pending();
+
+  if (!inChildProcess()) {
+    cache_storage = getCacheStorage("disk") ;
+    wait_for_cache_index(asyncOpen);
+  } else {
+    asyncOpen();
+  }
+}
+
+function asyncOpen()
+{
+  var chan = make_channel(URL);
+
+  var cc = chan.QueryInterface(Ci.nsICacheInfoChannel);
+  cc.preferAlternativeDataType(altContentType, "", true);
+
+  chan.asyncOpen(new ChannelListener(readServerContent, null));
+}
+
+function readServerContent(request, buffer)
+{
+  var cc = request.QueryInterface(Ci.nsICacheInfoChannel);
+
+  Assert.equal(buffer, responseContent);
+  Assert.equal(cc.alternativeDataType, "");
+  check_has_alt_data_in_index(false);
+
+  if (!inChildProcess()) {
+    currentThread = Services.tm.currentThread;
+  }
+
+  executeSoon(() => {
+    var os = cc.openAlternativeOutputStream(altContentType, altContent.length);
+
+    var aos = os.QueryInterface(Ci.nsIAsyncOutputStream);
+    aos.asyncWait(_ => {
+      os.write(altContent, altContent.length);
+      aos.closeWithStatus(Cr.NS_ERROR_FAILURE);
+      executeSoon(flushAndReadServerContentAgain);
+    }, 0, 0, currentThread);
+  });
+}
+
+function flushAndReadServerContentAgain()
+{
+  // We need to do a GC pass to ensure the cache entry has been freed.
+  gc();
+  if (!inChildProcess()) {
+    Services.cache2.QueryInterface(Ci.nsICacheTesting).flush(cacheFlushObserver);
+  } else {
+    do_send_remote_message('flush');
+    do_await_remote_message('flushed').then(() => {
+      readServerContentAgain();
+    });
+  }
+}
+
+function readServerContentAgain() {
+  var chan = make_channel(URL);
+  var cc = chan.QueryInterface(Ci.nsICacheInfoChannel);
+  cc.preferAlternativeDataType("dummy1", "text/javascript", true);
+  cc.preferAlternativeDataType(altContentType, "text/plain", true);
+  cc.preferAlternativeDataType("dummy2", "", true);
+
+  chan.asyncOpen(new ChannelListener(readServerContentAgainCB, null));
+}
+
+function readServerContentAgainCB(request, buffer)
+{
+  var cc = request.QueryInterface(Ci.nsICacheInfoChannel);
+
+  Assert.equal(buffer, responseContent);
+  Assert.equal(cc.alternativeDataType, "");
+  check_has_alt_data_in_index(false);
+  httpServer.stop(do_test_finished);
+}

netwerk/test/unit/xpcshell.ini

@@ -389,6 +389,7 @@ skip-if = appname == "thunderbird"
 [test_alt-data_stream.js]
 [test_alt-data_too_big.js]
 [test_alt-data_overwrite.js]
+[test_alt-data_closeWithStatus.js]
 [test_cache-control_request.js]
 [test_bug1279246.js]
 [test_throttlequeue.js]

netwerk/test/unit_ipc/test_alt-data_closeWithStatus_wrap.js

@@ -0,0 +1,17 @@
+const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+
+// needs to be rooted
+var cacheFlushObserver = { observe: function() {
+  cacheFlushObserver = null;
+  do_send_remote_message('flushed');
+}};
+
+var currentThread = Services.tm.currentThread;
+
+function run_test() {
+  do_get_profile();
+  do_await_remote_message('flush').then(() => {
+    Services.cache2.QueryInterface(Ci.nsICacheTesting).flush(cacheFlushObserver);
+  });
+  run_test_in_child("../unit/test_alt-data_closeWithStatus.js");
+}

netwerk/test/unit_ipc/xpcshell.ini

@@ -53,6 +53,7 @@ support-files =
   !/netwerk/test/unit/data/signed_win.exe
   !/netwerk/test/unit/test_alt-data_simple.js
   !/netwerk/test/unit/test_alt-data_stream.js
+  !/netwerk/test/unit/test_alt-data_closeWithStatus.js
   !/netwerk/test/unit/test_channel_priority.js
   !/netwerk/test/unit/test_multipart_streamconv.js
   !/netwerk/test/unit/test_original_sent_received_head.js
@@ -98,6 +99,7 @@ skip-if = true
 [test_getHost_wrap.js]
 [test_alt-data_simple_wrap.js]
 [test_alt-data_stream_wrap.js]
+[test_alt-data_closeWithStatus_wrap.js]
 [test_original_sent_received_head_wrap.js]
 [test_channel_id.js]
 [test_trackingProtection_annotateChannels_wrap1.js]