Bug 1397257 - [Windows] Awesome Screenshot removing error for a second uninstallation. r=jimm, a=lizzard
authorHaik Aftandilian <haftandilian@mozilla.com>
Thu, 07 Sep 2017 17:34:18 -0700
changeset 424094 fafd69ca273333772746a9c54ec7975a6ab1bc42
parent 424093 58e1249a4920875a7935730b61b788dc3d7288ff
child 424095 776744d895c98d89764bca8018509a77a2127cab
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm, lizzard
bugs1397257
milestone56.0
Bug 1397257 - [Windows] Awesome Screenshot removing error for a second uninstallation. r=jimm, a=lizzard 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
@@ -107,17 +107,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
@@ -125,43 +124,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();
@@ -184,20 +165,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),
@@ -263,33 +240,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(
@@ -778,40 +743,69 @@ ExtensionProtocolHandler::NewFD(nsIURI* 
     mozilla::NewRunnableMethod("ExtensionJarFileOpener",
         fileOpener, &ExtensionJARFileOpener::OpenFile);
 
   NS_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)
@@ -870,35 +864,41 @@ ExtensionProtocolHandler::SubstituteRemo
   NS_TRY(rv);
 
   bool isCached = false;
   NS_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;
-    NS_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);
-    NS_TRY(rv);
+  nsCOMPtr<nsIURI> innerFileURI;
+  NS_TRY(jarURI->GetJARFile(getter_AddRefs(innerFileURI)));
 
-    nsCOMPtr<nsIFile> jarFile;
-    NS_TRY(innerFileURL->GetFile(getter_AddRefs(jarFile)));
+  nsCOMPtr<nsIFileURL> innerFileURL = do_QueryInterface(innerFileURI, &rv);
+  NS_TRY(rv);
 
-    streamGetter = new ExtensionStreamGetter(aURI,
-                                             aLoadinfo,
-                                             jarChannel.forget(),
-                                             jarFile);
-  }
+  nsCOMPtr<nsIFile> jarFile;
+  NS_TRY(innerFileURL->GetFile(getter_AddRefs(jarFile)));
+
+  RefPtr<ExtensionStreamGetter> streamGetter =
+    new ExtensionStreamGetter(aURI,
+                              aLoadinfo,
+                              jarChannel.forget(),
+                              jarFile);
 
   NewSimpleChannel(aURI, aLoadinfo, streamGetter, aRetVal);
   return Ok();
 }
 
 #undef NS_TRY
 
 } // namespace net