Bug 1382783 - Retarget non-HTTP image URIs (chrome, blob) to the image IO thread if not an SVG. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Mon, 25 Sep 2017 11:44:49 -0400
changeset 382806 db42206dd4498e112ffad99103b8ceaf0af3a0be
parent 382805 e8d213d3265f8d6cb0401b2c2b127b4b5908baf5
child 382807 ebd3c3b2d64442c2b5eb7ab3e87c4b423311f3f4
push id32575
push userkwierso@gmail.com
push dateMon, 25 Sep 2017 23:41:46 +0000
treeherdermozilla-central@e6b3498a39b9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1382783, 867755, 1325080
milestone58.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 1382783 - Retarget non-HTTP image URIs (chrome, blob) to the image IO thread if not an SVG. r=tnikkel Currently we only permit requests from HTTP channels to be retargeted to the image IO thread. It was implemented this way originally in bug 867755 but it does not appear there was a specific reason for that. The only kink in this is some browser chrome mochitests listen on debug build only events to ensure certain chrome images are loaded and/or drawn. As such, this patch ensures that those observer notifications continue to be served, requiring a dispatch from the image IO thread to the main thread. Another issue to note is that SVGs must be processed on the main thread; the underlying SVG document can only be accessed from it. We enforce this by checking the content type. The possibility already exists that an HTTP response could contain the wrong content type, and in that case, we fail to decode the image, as there is no content sniffing support for SVG. Thus there should be no additional risk taken by using the image IO thread from other non-HTTP channels (if they don't specify the SVG content type, it is not rendered today, and if they do, it will remain on the main thread as it is today). We also ignore data URIs. The specification requires that we process these images sychronously. See bug 1325080 for details.
image/ImageFactory.cpp
image/RasterImage.cpp
image/imgRequest.cpp
image/imgRequest.h
--- a/image/ImageFactory.cpp
+++ b/image/ImageFactory.cpp
@@ -81,40 +81,60 @@ ComputeImageFlags(ImageURL* uri, const n
   rv = uri->SchemeIs("data", &isDataURI);
   if (NS_SUCCEEDED(rv) && isDataURI) {
     imageFlags |= Image::INIT_FLAG_SYNC_LOAD;
   }
 
   return imageFlags;
 }
 
+#ifdef DEBUG
+static void
+NotifyImageLoading(ImageURL* aURI)
+{
+  if (!NS_IsMainThread()) {
+    RefPtr<ImageURL> uri(aURI);
+    nsCOMPtr<nsIRunnable> ev =
+      NS_NewRunnableFunction("NotifyImageLoading", [uri] () -> void {
+        NotifyImageLoading(uri);
+    });
+    SystemGroup::Dispatch(TaskCategory::Other, ev.forget());
+    return;
+  }
+
+  nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
+  NS_WARNING_ASSERTION(obs, "Can't get an observer service handle");
+  if (obs) {
+    nsAutoCString spec;
+    aURI->GetSpec(spec);
+    obs->NotifyObservers(nullptr, "image-loading", NS_ConvertUTF8toUTF16(spec).get());
+  }
+}
+#endif
+
 /* static */ already_AddRefed<Image>
 ImageFactory::CreateImage(nsIRequest* aRequest,
                           ProgressTracker* aProgressTracker,
                           const nsCString& aMimeType,
                           ImageURL* aURI,
                           bool aIsMultiPart,
                           uint32_t aInnerWindowId)
 {
   MOZ_ASSERT(gfxPrefs::SingletonExists(),
              "Pref observers should have been initialized already");
 
   // Compute the image's initialization flags.
   uint32_t imageFlags = ComputeImageFlags(aURI, aMimeType, aIsMultiPart);
 
 #ifdef DEBUG
   // Record the image load for startup performance testing.
-  if (NS_IsMainThread()) {
-    nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
-    NS_WARNING_ASSERTION(obs, "Can't get an observer service handle");
-    if (obs) {
-      nsAutoCString spec;
-      aURI->GetSpec(spec);
-      obs->NotifyObservers(nullptr, "image-loading", NS_ConvertUTF8toUTF16(spec).get());
-    }
+  bool match = false;
+  if ((NS_SUCCEEDED(aURI->SchemeIs("resource", &match)) && match) ||
+      (NS_SUCCEEDED(aURI->SchemeIs("chrome", &match)) && match)) {
+    NotifyImageLoading(aURI);
   }
 #endif
 
   // Select the type of image to create based on MIME type.
   if (aMimeType.EqualsLiteral(IMAGE_SVG_XML)) {
     return CreateVectorImage(aRequest, aProgressTracker, aMimeType,
                              aURI, imageFlags, aInnerWindowId);
   } else {
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -1695,16 +1695,22 @@ RasterImage::GetFramesNotified(uint32_t*
 #ifdef DEBUG
 void
 RasterImage::NotifyDrawingObservers()
 {
   if (!mURI || !NS_IsMainThread()) {
     return;
   }
 
+  bool match = false;
+  if ((NS_FAILED(mURI->SchemeIs("resource", &match)) || !match) &&
+      (NS_FAILED(mURI->SchemeIs("chrome", &match)) || !match)) {
+    return;
+  }
+
   // Record the image drawing for startup performance testing.
   nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
   NS_WARNING_ASSERTION(obs, "Can't get an observer service handle");
   if (obs) {
     nsCOMPtr<nsIURI> imageURI = mURI->ToIURI();
     nsAutoCString spec;
     imageURI->GetSpec(spec);
     obs->NotifyObservers(nullptr, "image-drawing", NS_ConvertUTF8toUTF16(spec).get());
--- a/image/imgRequest.cpp
+++ b/image/imgRequest.cpp
@@ -454,23 +454,36 @@ imgRequest::GetCurrentURI(nsIURI** aURI)
     NS_ADDREF(*aURI);
     return NS_OK;
   }
 
   return NS_ERROR_FAILURE;
 }
 
 bool
+imgRequest::IsScheme(const char* aScheme) const
+{
+  MOZ_ASSERT(aScheme);
+  bool isScheme = false;
+  if (NS_WARN_IF(NS_FAILED(mURI->SchemeIs(aScheme, &isScheme)))) {
+    return false;
+  }
+  return isScheme;
+}
+
+bool
 imgRequest::IsChrome() const
 {
-  bool isChrome = false;
-  if (NS_WARN_IF(NS_FAILED(mURI->SchemeIs("chrome", &isChrome)))) {
-    return false;
-  }
-  return isChrome;
+  return IsScheme("chrome");
+}
+
+bool
+imgRequest::IsData() const
+{
+  return IsScheme("data");
 }
 
 nsresult
 imgRequest::GetImageErrorCode()
 {
   return mImageErrorCode;
 }
 
@@ -814,23 +827,27 @@ imgRequest::OnStartRequest(nsIRequest* a
 
   // Shouldn't we be dead already if this gets hit?
   // Probably multipart/x-mixed-replace...
   RefPtr<ProgressTracker> progressTracker = GetProgressTracker();
   if (progressTracker->ObserverCount() == 0) {
     this->Cancel(NS_IMAGELIB_ERROR_FAILURE);
   }
 
-  // Try to retarget OnDataAvailable to a decode thread.
-  nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aRequest);
+  // Try to retarget OnDataAvailable to a decode thread. We must process data
+  // URIs synchronously as per the spec however.
+  if (!channel || IsData()) {
+    return NS_OK;
+  }
+
   nsCOMPtr<nsIThreadRetargetableRequest> retargetable =
     do_QueryInterface(aRequest);
-  if (httpChannel && retargetable) {
+  if (retargetable) {
     nsAutoCString mimeType;
-    nsresult rv = httpChannel->GetContentType(mimeType);
+    nsresult rv = channel->GetContentType(mimeType);
     if (NS_SUCCEEDED(rv) && !mimeType.EqualsLiteral(IMAGE_SVG_XML)) {
       // Retarget OnDataAvailable to the DecodePool's IO thread.
       nsCOMPtr<nsIEventTarget> target =
         DecodePool::Singleton()->GetIOEventTarget();
       rv = retargetable->RetargetDeliveryTo(target);
     }
     MOZ_LOG(gImgLog, LogLevel::Warning,
            ("[this=%p] imgRequest::OnStartRequest -- "
--- a/image/imgRequest.h
+++ b/image/imgRequest.h
@@ -148,17 +148,19 @@ public:
   const ImageCacheKey& CacheKey() const { return mCacheKey; }
 
   // Resize the cache entry to 0 if it exists
   void ResetCacheEntry();
 
   // OK to use on any thread.
   nsresult GetURI(ImageURL** aURI);
   nsresult GetCurrentURI(nsIURI** aURI);
+  bool IsScheme(const char* aScheme) const;
   bool IsChrome() const;
+  bool IsData() const;
 
   nsresult GetImageErrorCode(void);
 
   /// Returns true if we've received any data.
   bool HasTransferredData() const;
 
   /// Returns a non-owning pointer to this imgRequest's MIME type.
   const char* GetMimeType() const { return mContentType.get(); }