Bug 1397257 - [Windows] Awesome Screenshot removing error for a second uninstallation. r?jimm draft
authorhaftandilian@mozilla.com
Thu, 07 Sep 2017 17:34:18 -0700
changeset 662499 4d0cdfa43a062420610f4ec6bcdd73a5a0daca04
parent 662498 2f7e2c88ddb67f646f57cc5a20945c6f45cd7ee7
child 730886 d1df3374c6e8f7870e8e513d6577eaea83b6cb38
push id79101
push userhaftandilian@mozilla.com
push dateMon, 11 Sep 2017 18:54:48 +0000
reviewersjimm
bugs1397257
milestone57.0a1
Bug 1397257 - [Windows] Awesome Screenshot removing error for a second uninstallation. r?jimm Fix circular dependency created when encountering a path to a nonexistent JAR inner file. Change cache JAR loads to not use ExtensionStreamGetter, instead call the JAR channel's AsyncOpen2 method directly in the SimpleChannel callback. Remove the code to handle cached JAR loads from ExtensionStreamGetter. MozReview-Commit-ID: Kmry02eLYU1
netwerk/protocol/res/ExtensionProtocolHandler.cpp
--- a/netwerk/protocol/res/ExtensionProtocolHandler.cpp
+++ b/netwerk/protocol/res/ExtensionProtocolHandler.cpp
@@ -72,17 +72,16 @@ class ExtensionStreamGetter : public Ref
 {
   public:
     // To use when getting a remote input stream for a resource
     // in an unpacked extension.
     ExtensionStreamGetter(nsIURI* aURI, nsILoadInfo* aLoadInfo)
       : mURI(aURI)
       , mLoadInfo(aLoadInfo)
       , mIsJarChannel(false)
-      , mIsCachedJar(false)
     {
       MOZ_ASSERT(aURI);
       MOZ_ASSERT(aLoadInfo);
 
       SetupEventTarget();
     }
 
     // To use when getting an FD for a packed extension JAR file
@@ -90,43 +89,25 @@ class ExtensionStreamGetter : public Ref
     ExtensionStreamGetter(nsIURI* aURI, nsILoadInfo* aLoadInfo,
                           already_AddRefed<nsIJARChannel>&& aJarChannel,
                           nsIFile* aJarFile)
       : mURI(aURI)
       , mLoadInfo(aLoadInfo)
       , mJarChannel(Move(aJarChannel))
       , mJarFile(aJarFile)
       , mIsJarChannel(true)
-      , mIsCachedJar(false)
     {
       MOZ_ASSERT(aURI);
       MOZ_ASSERT(aLoadInfo);
       MOZ_ASSERT(mJarChannel);
       MOZ_ASSERT(aJarFile);
 
       SetupEventTarget();
     }
 
-    // To use when the request resolves to a JAR file that is already cached.
-    // Using a SimpleChannel with an ExtensionStreamGetter here (like the
-    // non-cached JAR case) isn't needed to load the extension resource
-    // because we don't need to ask the parent for an FD for the JAR, but
-    // wrapping the JARChannel in a SimpleChannel allows HTTP forwarding to
-    // moz-extension URI's to work because HTTP forwarding requires the
-    // target channel implement nsIChildChannel. mMainThreadEventTarget is
-    // not used for this case, so don't set it up.
-    explicit
-      ExtensionStreamGetter(already_AddRefed<nsIJARChannel>&& aJarChannel)
-      : mJarChannel(Move(aJarChannel))
-      , mIsJarChannel(true)
-      , mIsCachedJar(true)
-    {
-      MOZ_ASSERT(mJarChannel);
-    }
-
     ~ExtensionStreamGetter() {}
 
     void SetupEventTarget()
     {
       mMainThreadEventTarget =
         nsContentUtils::GetEventTargetByLoadInfo(mLoadInfo, TaskCategory::Other);
       if (!mMainThreadEventTarget) {
         mMainThreadEventTarget = GetMainThreadSerialEventTarget();
@@ -149,20 +130,16 @@ class ExtensionStreamGetter : public Ref
     nsCOMPtr<nsIURI> mURI;
     nsCOMPtr<nsILoadInfo> mLoadInfo;
     nsCOMPtr<nsIJARChannel> mJarChannel;
     nsCOMPtr<nsIFile> mJarFile;
     nsCOMPtr<nsIStreamListener> mListener;
     nsCOMPtr<nsIChannel> mChannel;
     nsCOMPtr<nsISerialEventTarget> mMainThreadEventTarget;
     bool mIsJarChannel;
-
-    // Indicates the JAR for this channel is cached
-    // and implies mIsJarChannel==true
-    bool mIsCachedJar;
 };
 
 class ExtensionJARFileOpener : public nsISupports
 {
 public:
   ExtensionJARFileOpener(nsIFile* aFile,
                          NeckoParent::GetExtensionFDResolver& aResolve) :
     mFile(aFile),
@@ -228,33 +205,21 @@ NS_IMPL_ISUPPORTS(ExtensionJARFileOpener
 #define DEFAULT_THREAD_TIMEOUT_MS 30000
 
 // Request an FD or input stream from the parent.
 Result<Ok, nsresult>
 ExtensionStreamGetter::GetAsync(nsIStreamListener* aListener,
                                 nsIChannel* aChannel)
 {
   MOZ_ASSERT(IsNeckoChild());
-  MOZ_ASSERT(mMainThreadEventTarget || mIsCachedJar);
+  MOZ_ASSERT(mMainThreadEventTarget);
 
   mListener = aListener;
   mChannel = aChannel;
 
-  // We don't have to request an FD from the
-  // parent if the JAR is cached
-  if (mIsCachedJar) {
-    MOZ_ASSERT(mIsJarChannel);
-    nsresult rv = mJarChannel->AsyncOpen2(mListener);
-    if (NS_FAILED(rv)) {
-      mChannel->Cancel(NS_BINDING_ABORTED);
-      return Result<Ok, nsresult>(rv);
-    }
-    return Ok();
-  }
-
   // Serialize the URI to send to parent
   mozilla::ipc::URIParams uri;
   SerializeURI(mURI, uri);
 
   RefPtr<ExtensionStreamGetter> self = this;
   if (mIsJarChannel) {
     // Request an FD for this moz-extension URI
     gNeckoChild->SendGetExtensionFD(uri)->Then(
@@ -822,40 +787,69 @@ ExtensionProtocolHandler::NewFD(nsIURI* 
     mozilla::NewRunnableMethod("ExtensionJarFileOpener",
         fileOpener, &ExtensionJARFileOpener::OpenFile);
 
   MOZ_TRY(mFileOpenerThread->Dispatch(event, nsIEventTarget::DISPATCH_NORMAL));
 
   return Ok();
 }
 
+// Set the channel's content type using the provided URI's type
+void
+SetContentType(nsIURI* aURI, nsIChannel* aChannel)
+{
+  nsresult rv;
+  nsCOMPtr<nsIMIMEService> mime = do_GetService("@mozilla.org/mime;1", &rv);
+  if (NS_SUCCEEDED(rv)) {
+    nsAutoCString contentType;
+    rv = mime->GetTypeFromURI(aURI, contentType);
+    if (NS_SUCCEEDED(rv)) {
+      Unused << aChannel->SetContentType(contentType);
+    }
+  }
+}
+
+// Gets a SimpleChannel that wraps the provided ExtensionStreamGetter
 static void
 NewSimpleChannel(nsIURI* aURI,
                  nsILoadInfo* aLoadinfo,
                  ExtensionStreamGetter* aStreamGetter,
                  nsIChannel** aRetVal)
 {
   nsCOMPtr<nsIChannel> channel = NS_NewSimpleChannel(
     aURI, aLoadinfo, aStreamGetter,
-    [] (nsIStreamListener* listener, nsIChannel* channel,
+    [] (nsIStreamListener* listener, nsIChannel* simpleChannel,
         ExtensionStreamGetter* getter) -> RequestOrReason {
-      MOZ_TRY(getter->GetAsync(listener, channel));
+      MOZ_TRY(getter->GetAsync(listener, simpleChannel));
       return RequestOrReason(nullptr);
     });
 
-  nsresult rv;
-  nsCOMPtr<nsIMIMEService> mime = do_GetService("@mozilla.org/mime;1", &rv);
-  if (NS_SUCCEEDED(rv)) {
-    nsAutoCString contentType;
-    rv = mime->GetTypeFromURI(aURI, contentType);
-    if (NS_SUCCEEDED(rv)) {
-      Unused << channel->SetContentType(contentType);
-    }
-  }
+  SetContentType(aURI, channel);
+  channel.swap(*aRetVal);
+}
 
+// Gets a SimpleChannel that wraps the provided channel
+static void
+NewSimpleChannel(nsIURI* aURI,
+                 nsILoadInfo* aLoadinfo,
+                 nsIChannel* aChannel,
+                 nsIChannel** aRetVal)
+{
+  nsCOMPtr<nsIChannel> channel = NS_NewSimpleChannel(aURI, aLoadinfo, aChannel,
+    [] (nsIStreamListener* listener, nsIChannel* simpleChannel,
+        nsIChannel* origChannel) -> RequestOrReason {
+      nsresult rv = origChannel->AsyncOpen2(listener);
+      if (NS_FAILED(rv)) {
+        simpleChannel->Cancel(NS_BINDING_ABORTED);
+        return RequestOrReason(rv);
+      }
+      return RequestOrReason(origChannel);
+    });
+
+  SetContentType(aURI, channel);
   channel.swap(*aRetVal);
 }
 
 void
 ExtensionProtocolHandler::SubstituteRemoteFileChannel(nsIURI* aURI,
                                                       nsILoadInfo* aLoadinfo,
                                                       nsACString& aResolvedFileSpec,
                                                       nsIChannel** aRetVal)
@@ -914,34 +908,40 @@ ExtensionProtocolHandler::SubstituteRemo
   MOZ_TRY(rv);
 
   bool isCached = false;
   MOZ_TRY(jarChannel->EnsureCached(&isCached));
   if (MOZ_LOG_TEST(gExtProtocolLog, LogLevel::Debug)) {
     Unused << LogCacheCheck(jarChannel, jarURI, isCached);
   }
 
-  RefPtr<ExtensionStreamGetter> streamGetter;
-
   if (isCached) {
-    streamGetter = new ExtensionStreamGetter(jarChannel.forget());
-  } else {
-    nsCOMPtr<nsIURI> innerFileURI;
-    MOZ_TRY(jarURI->GetJARFile(getter_AddRefs(innerFileURI)));
+    // Using a SimpleChannel with an ExtensionStreamGetter here (like the
+    // non-cached JAR case) isn't needed to load the extension resource
+    // because we don't need to ask the parent for an FD for the JAR, but
+    // wrapping the JARChannel in a SimpleChannel allows HTTP forwarding to
+    // moz-extension URI's to work because HTTP forwarding requires the
+    // target channel implement nsIChildChannel.
+    NewSimpleChannel(aURI, aLoadinfo, jarChannel.get(), aRetVal);
+    return Ok();
+  }
 
-    nsCOMPtr<nsIFileURL> innerFileURL = do_QueryInterface(innerFileURI, &rv);
-    MOZ_TRY(rv);
+  nsCOMPtr<nsIURI> innerFileURI;
+  MOZ_TRY(jarURI->GetJARFile(getter_AddRefs(innerFileURI)));
 
-    nsCOMPtr<nsIFile> jarFile;
-    MOZ_TRY(innerFileURL->GetFile(getter_AddRefs(jarFile)));
+  nsCOMPtr<nsIFileURL> innerFileURL = do_QueryInterface(innerFileURI, &rv);
+  MOZ_TRY(rv);
 
-    streamGetter = new ExtensionStreamGetter(aURI,
-                                             aLoadinfo,
-                                             jarChannel.forget(),
-                                             jarFile);
-  }
+  nsCOMPtr<nsIFile> jarFile;
+  MOZ_TRY(innerFileURL->GetFile(getter_AddRefs(jarFile)));
+
+  RefPtr<ExtensionStreamGetter> streamGetter =
+    new ExtensionStreamGetter(aURI,
+                              aLoadinfo,
+                              jarChannel.forget(),
+                              jarFile);
 
   NewSimpleChannel(aURI, aLoadinfo, streamGetter, aRetVal);
   return Ok();
 }
 
 } // namespace net
 } // namespace mozilla