Bug 1231213 - Correctly choose nsBufferedStream's underlying nsIInputStream after initialization. r=asuth
☠☠ backed out by 3cf55b7f12f2 ☠ ☠
authorPerry Jiang <perry@mozilla.com>
Wed, 14 Aug 2019 16:19:33 +0000
changeset 487965 3449c2eba4c6b196fc3f0be090a5aa48058b8173
parent 487964 ae221b6288996b15104e52c497e2cae59da9f536
child 487966 2d5628a0e52dc9aff4a67fa05da6d238793f9af6
push id36434
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:44:30 +0000
treeherdermozilla-central@144fbfb409b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1231213
milestone70.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 1231213 - Correctly choose nsBufferedStream's underlying nsIInputStream after initialization. r=asuth Differential Revision: https://phabricator.services.mozilla.com/D26159
netwerk/base/nsBufferedStreams.cpp
netwerk/base/nsBufferedStreams.h
netwerk/base/nsNetUtil.cpp
--- a/netwerk/base/nsBufferedStreams.cpp
+++ b/netwerk/base/nsBufferedStreams.cpp
@@ -4,16 +4,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ipc/IPCMessageUtils.h"
 
 #include "nsBufferedStreams.h"
 #include "nsStreamUtils.h"
 #include "nsNetCID.h"
 #include "nsIClassInfoImpl.h"
+#include "mozilla/DebugOnly.h"
 #include "mozilla/ipc/InputStreamUtils.h"
 #include <algorithm>
 
 #ifdef DEBUG_brendan
 #  define METERING
 #endif
 
 #ifdef METERING
@@ -33,16 +34,17 @@ static struct {
     int64_t mNewOffset;
   } mBigSeek[MAX_BIG_SEEKS];
 } bufstats;
 #else
 #  define METER(x) /* nothing */
 #endif
 
 using namespace mozilla::ipc;
+using mozilla::DebugOnly;
 using mozilla::Maybe;
 using mozilla::MutexAutoLock;
 using mozilla::Nothing;
 using mozilla::Some;
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsBufferedStream
 
@@ -271,17 +273,26 @@ nsresult nsBufferedStream::GetData(nsISu
 
 NS_IMPL_ADDREF_INHERITED(nsBufferedInputStream, nsBufferedStream)
 NS_IMPL_RELEASE_INHERITED(nsBufferedInputStream, nsBufferedStream)
 
 NS_IMPL_CLASSINFO(nsBufferedInputStream, nullptr, nsIClassInfo::THREADSAFE,
                   NS_BUFFEREDINPUTSTREAM_CID)
 
 NS_INTERFACE_MAP_BEGIN(nsBufferedInputStream)
-  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsIInputStream, nsIBufferedInputStream)
+  // Unfortunately there isn't a macro that combines ambiguous and conditional,
+  // and as far as I can tell, no other class would need such a macro.
+  if (mIsAsyncInputStream && aIID.Equals(NS_GET_IID(nsIInputStream))) {
+    foundInterface =
+        static_cast<nsIInputStream*>(static_cast<nsIAsyncInputStream*>(this));
+  } else if (!mIsAsyncInputStream && aIID.Equals(NS_GET_IID(nsIInputStream))) {
+    foundInterface = static_cast<nsIInputStream*>(
+        static_cast<nsIBufferedInputStream*>(this));
+  } else
+    NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIBufferedInputStream)
   NS_INTERFACE_MAP_ENTRY(nsIBufferedInputStream)
   NS_INTERFACE_MAP_ENTRY(nsIStreamBufferAccess)
   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIIPCSerializableInputStream,
                                      mIsIPCSerializable)
   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIAsyncInputStream, mIsAsyncInputStream)
   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIInputStreamCallback,
                                      mIsAsyncInputStream)
   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsICloneableInputStream,
@@ -343,16 +354,29 @@ nsBufferedInputStream::Init(nsIInputStre
   {
     nsCOMPtr<nsIAsyncInputStreamLength> stream = do_QueryInterface(mStream);
     mIsAsyncInputStreamLength = !!stream;
   }
 
   return NS_OK;
 }
 
+already_AddRefed<nsIInputStream> nsBufferedInputStream::GetInputStream() {
+  // A non-null mStream implies Init() has been called.
+  MOZ_ASSERT(mStream);
+
+  nsIInputStream* out = nullptr;
+  DebugOnly<nsresult> rv = QueryInterface(NS_GET_IID(nsIInputStream),
+                                          reinterpret_cast<void**>(&out));
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
+  MOZ_ASSERT(out);
+
+  return already_AddRefed<nsIInputStream>(out);
+}
+
 NS_IMETHODIMP
 nsBufferedInputStream::Close() {
   nsresult rv1 = NS_OK, rv2;
   if (mStream) {
     rv1 = Source()->Close();
 #ifdef DEBUG
     if (NS_FAILED(rv1)) {
       NS_WARNING(
@@ -759,17 +783,19 @@ nsBufferedInputStream::Clone(nsIInputStr
   nsCOMPtr<nsIInputStream> clonedStream;
   nsresult rv = stream->Clone(getter_AddRefs(clonedStream));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIBufferedInputStream> bis = new nsBufferedInputStream();
   rv = bis->Init(clonedStream, mBufferSize);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  bis.forget(aResult);
+  *aResult =
+      static_cast<nsBufferedInputStream*>(bis.get())->GetInputStream().take();
+
   return NS_OK;
 }
 
 // nsIInputStreamLength
 
 NS_IMETHODIMP
 nsBufferedInputStream::Length(int64_t* aLength) {
   nsCOMPtr<nsIInputStreamLength> stream = do_QueryInterface(mStream);
--- a/netwerk/base/nsBufferedStreams.h
+++ b/netwerk/base/nsBufferedStreams.h
@@ -87,16 +87,32 @@ class nsBufferedInputStream final : publ
   NS_DECL_NSIINPUTSTREAMLENGTHCALLBACK
 
   nsBufferedInputStream();
 
   static nsresult Create(nsISupports* aOuter, REFNSIID aIID, void** aResult);
 
   nsIInputStream* Source() { return (nsIInputStream*)mStream.get(); }
 
+  /**
+   * If there's a reference/pointer to an nsBufferedInputStream BEFORE calling
+   * Init() AND the intent is to ultimately convert/assign that
+   * reference/pointer to an nsIInputStream, DO NOT use that initial
+   * reference/pointer. Instead, use the value of QueryInterface-ing to an
+   * nsIInputStream (and, again, the QueryInterface must be performed after
+   * Init()). This is because nsBufferedInputStream has multiple underlying
+   * nsIInputStreams (one from nsIBufferedInputStream and one from
+   * nsIAsyncInputStream), and the correct base nsIInputStream to use will be
+   * unknown until the final value of mIsAsyncInputStream is set in Init().
+   *
+   * This method, however, does just that but also hides the QI details and
+   * will assert if called before Init().
+   */
+  already_AddRefed<nsIInputStream> GetInputStream();
+
  protected:
   virtual ~nsBufferedInputStream() = default;
 
   template <typename M>
   void SerializeInternal(mozilla::ipc::InputStreamParams& aParams,
                          FileDescriptorArray& aFileDescriptors,
                          bool aDelayedStart, uint32_t aMaxSize,
                          uint32_t* aSizeUsed, M* aManager);
--- a/netwerk/base/nsNetUtil.cpp
+++ b/netwerk/base/nsNetUtil.cpp
@@ -25,16 +25,17 @@
 #include "nsFileStreams.h"
 #include "nsHashKeys.h"
 #include "nsHttp.h"
 #include "nsIAsyncStreamCopier.h"
 #include "nsIAuthPrompt.h"
 #include "nsIAuthPrompt2.h"
 #include "nsIAuthPromptAdapterFactory.h"
 #include "nsIBufferedStreams.h"
+#include "nsBufferedStreams.h"
 #include "nsIChannelEventSink.h"
 #include "nsIContentSniffer.h"
 #include "mozilla/dom/Document.h"
 #include "nsICookieService.h"
 #include "nsIDownloader.h"
 #include "nsIFileProtocolHandler.h"
 #include "nsIFileStreams.h"
 #include "nsIFileURL.h"
@@ -1264,17 +1265,19 @@ MOZ_MUST_USE nsresult NS_NewBufferedInpu
   nsCOMPtr<nsIInputStream> inputStream = std::move(aInputStream);
 
   nsresult rv;
   nsCOMPtr<nsIBufferedInputStream> in =
       do_CreateInstance(NS_BUFFEREDINPUTSTREAM_CONTRACTID, &rv);
   if (NS_SUCCEEDED(rv)) {
     rv = in->Init(inputStream, aBufferSize);
     if (NS_SUCCEEDED(rv)) {
-      in.forget(aResult);
+      *aResult = static_cast<nsBufferedInputStream*>(in.get())
+                     ->GetInputStream()
+                     .take();
     }
   }
   return rv;
 }
 
 namespace {
 
 #define BUFFER_SIZE 8192