Bug 1487113 - AltDataOutputStreamChild must be nsIAsyncOutputStream, r=valentin
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 27 Feb 2019 20:50:48 +0000
changeset 519409 169c6e292149904e7e1afb3456cb7b8ba253500c
parent 519408 de6dc58f6f51caf627274eef936c7dbd2df69823
child 519410 759ec998fe87daa4c1fd49608fe35fabd01239a5
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1487113
milestone67.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 1487113 - AltDataOutputStreamChild must be nsIAsyncOutputStream, r=valentin Differential Revision: https://phabricator.services.mozilla.com/D21379
netwerk/protocol/http/AltDataOutputStreamChild.cpp
netwerk/protocol/http/AltDataOutputStreamChild.h
netwerk/protocol/http/AltDataOutputStreamParent.cpp
netwerk/protocol/http/AltDataOutputStreamParent.h
netwerk/protocol/http/PAltDataOutputStream.ipdl
netwerk/test/unit/test_alt-data_closeWithStatus.js
netwerk/test/unit/xpcshell.ini
netwerk/test/unit_ipc/test_alt-data_closeWithStatus_wrap.js
netwerk/test/unit_ipc/xpcshell.ini
--- a/netwerk/protocol/http/AltDataOutputStreamChild.cpp
+++ b/netwerk/protocol/http/AltDataOutputStreamChild.cpp
@@ -1,11 +1,12 @@
 #include "mozilla/net/AltDataOutputStreamChild.h"
 #include "mozilla/Unused.h"
 #include "nsIInputStream.h"
+#include "nsStreamUtils.h"
 
 namespace mozilla {
 namespace net {
 
 NS_IMPL_ADDREF(AltDataOutputStreamChild)
 
 NS_IMETHODIMP_(MozExternalRefCountType) AltDataOutputStreamChild::Release() {
   MOZ_ASSERT(0 != mRefCnt, "dup release");
@@ -25,34 +26,40 @@ NS_IMETHODIMP_(MozExternalRefCountType) 
     mRefCnt = 1; /* stabilize */
     delete this;
     return 0;
   }
   return mRefCnt;
 }
 
 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");
 }
 
 void AltDataOutputStreamChild::AddIPDLReference() {
   MOZ_ASSERT(!mIPCOpen, "Attempt to retain more than one IPDL reference");
   mIPCOpen = true;
   AddRef();
 }
 
 void AltDataOutputStreamChild::ReleaseIPDLReference() {
   MOZ_ASSERT(mIPCOpen, "Attempt to release nonexistent IPDL reference");
   mIPCOpen = false;
+
+  if (mCallback) {
+    NotifyListener();
+  }
+
   Release();
 }
 
 bool AltDataOutputStreamChild::WriteDataInChunks(
     const nsDependentCSubstring &data) {
   const uint32_t kChunkSize = 128 * 1024;
   uint32_t next = std::min(data.Length(), kChunkSize);
   for (uint32_t i = 0; i < data.Length();
@@ -62,26 +69,17 @@ bool AltDataOutputStreamChild::WriteData
       mIPCOpen = false;
       return false;
     }
   }
   return true;
 }
 
 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() {
   if (!mIPCOpen) {
     return NS_ERROR_NOT_AVAILABLE;
   }
   if (NS_FAILED(mError)) {
     return mError;
@@ -132,10 +130,61 @@ mozilla::ipc::IPCResult AltDataOutputStr
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult AltDataOutputStreamChild::RecvDeleteSelf() {
   PAltDataOutputStreamChild::Send__delete__(this);
   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
--- a/netwerk/protocol/http/AltDataOutputStreamChild.h
+++ b/netwerk/protocol/http/AltDataOutputStreamChild.h
@@ -4,42 +4,48 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_net_AltDataOutputStreamChild_h
 #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();
 
   void AddIPDLReference();
   void ReleaseIPDLReference();
   // Saves an error code which will be reported to the writer on the next call.
   virtual mozilla::ipc::IPCResult RecvError(const nsresult& err);
   virtual mozilla::ipc::IPCResult RecvDeleteSelf();
 
  private:
   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
 }  // namespace mozilla
 
 #endif  // mozilla_net_AltDataOutputStreamChild_h
--- a/netwerk/protocol/http/AltDataOutputStreamParent.cpp
+++ b/netwerk/protocol/http/AltDataOutputStreamParent.cpp
@@ -37,31 +37,39 @@ mozilla::ipc::IPCResult AltDataOutputStr
     MOZ_ASSERT(n == data.Length() || NS_FAILED(rv));
     if (NS_FAILED(rv) && mIPCOpen) {
       Unused << SendError(rv);
     }
   }
   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();
 }
 
 void AltDataOutputStreamParent::ActorDestroy(ActorDestroyReason aWhy) {
   mIPCOpen = false;
 }
 
 mozilla::ipc::IPCResult AltDataOutputStreamParent::RecvDeleteSelf() {
--- a/netwerk/protocol/http/AltDataOutputStreamParent.h
+++ b/netwerk/protocol/http/AltDataOutputStreamParent.h
@@ -25,17 +25,17 @@ class AltDataOutputStreamParent : public
   // aStream may be null
   explicit AltDataOutputStreamParent(nsIOutputStream* aStream);
 
   // Called when data is received from the content process.
   // We proceed to write that data to the output stream.
   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.
   void SetError(nsresult status) { mStatus = status; }
   mozilla::ipc::IPCResult RecvDeleteSelf();
 
  private:
   virtual ~AltDataOutputStreamParent();
--- a/netwerk/protocol/http/PAltDataOutputStream.ipdl
+++ b/netwerk/protocol/http/PAltDataOutputStream.ipdl
@@ -13,17 +13,17 @@ namespace net {
 protocol PAltDataOutputStream
 {
   manager PNecko;
 
 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__();
 
 child:
   // The parent calls this method to signal that an error has ocurred.
   // This may mean that opening the output stream has failed or that writing to
   // the stream has returned an error.
   async Error(nsresult err);
new file mode 100644
--- /dev/null
+++ b/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);
+}
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -384,16 +384,17 @@ skip-if = os == "android"
 [test_bug412457.js]
 skip-if = appname == "thunderbird"
 [test_bug464591.js]
 skip-if = appname == "thunderbird"
 [test_alt-data_simple.js]
 [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]
 [test_throttlechannel.js]
 [test_throttling.js]
 [test_separate_connections.js]
 [test_trackingProtection_annotateChannels.js]
 [test_race_cache_with_network.js]
new file mode 100644
--- /dev/null
+++ b/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");
+}
--- a/netwerk/test/unit_ipc/xpcshell.ini
+++ b/netwerk/test/unit_ipc/xpcshell.ini
@@ -48,16 +48,17 @@ support-files =
   !/netwerk/test/unit/data/test_readline4.txt
   !/netwerk/test/unit/data/test_readline5.txt
   !/netwerk/test/unit/data/test_readline6.txt
   !/netwerk/test/unit/data/test_readline7.txt
   !/netwerk/test/unit/data/test_readline8.txt
   !/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
   !/netwerk/test/unit/test_alt-data_cross_process.js
   !/netwerk/test/unit/test_crossOriginOpenerPolicy.js
   child_cookie_header.js
 
 [test_bug528292_wrap.js]
@@ -93,16 +94,17 @@ skip-if = true
 [test_synthesized_response_wrap.js]
 [test_xmlhttprequest_wrap.js]
 [test_XHR_redirects.js]
 [test_redirect_history_wrap.js]
 [test_reply_without_content_type_wrap.js]
 [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]
 [test_trackingProtection_annotateChannels_wrap2.js]
 [test_channel_priority_wrap.js]
 [test_multipart_streamconv_wrap.js]
 [test_alt-data_cross_process_wrap.js]
 [test_crossOriginOpenerPolicy_wrap.js]