From f67ab8c05706f2f0da83fa02d8ef378f0b0cf7ca Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Mon, 25 Sep 2017 11:44:49 -0400 Subject: [PATCH] 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 | 36 ++++++++++++++++++++++++++++-------- image/RasterImage.cpp | 6 ++++++ image/imgRequest.cpp | 33 +++++++++++++++++++++++++-------- image/imgRequest.h | 2 ++ 4 files changed, 61 insertions(+), 16 deletions(-) diff --git a/image/ImageFactory.cpp b/image/ImageFactory.cpp index 0abdf505bf1a..cb6cf35fa441 100644 --- a/image/ImageFactory.cpp +++ b/image/ImageFactory.cpp @@ -86,6 +86,30 @@ ComputeImageFlags(ImageURL* uri, const nsCString& aMimeType, bool isMultiPart) return imageFlags; } +#ifdef DEBUG +static void +NotifyImageLoading(ImageURL* aURI) +{ + if (!NS_IsMainThread()) { + RefPtr uri(aURI); + nsCOMPtr ev = + NS_NewRunnableFunction("NotifyImageLoading", [uri] () -> void { + NotifyImageLoading(uri); + }); + SystemGroup::Dispatch(TaskCategory::Other, ev.forget()); + return; + } + + nsCOMPtr 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 ImageFactory::CreateImage(nsIRequest* aRequest, ProgressTracker* aProgressTracker, @@ -102,14 +126,10 @@ ImageFactory::CreateImage(nsIRequest* aRequest, #ifdef DEBUG // Record the image load for startup performance testing. - if (NS_IsMainThread()) { - nsCOMPtr 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 diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index 918cfb9eead4..de35877c8e55 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -1700,6 +1700,12 @@ RasterImage::NotifyDrawingObservers() 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 obs = services::GetObserverService(); NS_WARNING_ASSERTION(obs, "Can't get an observer service handle"); diff --git a/image/imgRequest.cpp b/image/imgRequest.cpp index c96d601ec7eb..2e1442614e8a 100644 --- a/image/imgRequest.cpp +++ b/image/imgRequest.cpp @@ -459,13 +459,26 @@ imgRequest::GetCurrentURI(nsIURI** aURI) } bool -imgRequest::IsChrome() const +imgRequest::IsScheme(const char* aScheme) const { - bool isChrome = false; - if (NS_WARN_IF(NS_FAILED(mURI->SchemeIs("chrome", &isChrome)))) { + MOZ_ASSERT(aScheme); + bool isScheme = false; + if (NS_WARN_IF(NS_FAILED(mURI->SchemeIs(aScheme, &isScheme)))) { return false; } - return isChrome; + return isScheme; +} + +bool +imgRequest::IsChrome() const +{ + return IsScheme("chrome"); +} + +bool +imgRequest::IsData() const +{ + return IsScheme("data"); } nsresult @@ -819,13 +832,17 @@ imgRequest::OnStartRequest(nsIRequest* aRequest, nsISupports* ctxt) this->Cancel(NS_IMAGELIB_ERROR_FAILURE); } - // Try to retarget OnDataAvailable to a decode thread. - nsCOMPtr 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 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 target = diff --git a/image/imgRequest.h b/image/imgRequest.h index 4f8673578368..6b6c74864bf1 100644 --- a/image/imgRequest.h +++ b/image/imgRequest.h @@ -153,7 +153,9 @@ public: // 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);