Bug 1598480 - Make SVG images respond to theme changes. r=tnikkel,emilio

Differential Revision: https://phabricator.services.mozilla.com/D56156

--HG--
extra : moz-landing-system : lando

dom/base/ImageTracker.cpp

@@ -140,5 +140,29 @@ void ImageTracker::RequestDiscardAll() {
   }
 }
 
+void ImageTracker::MediaFeatureValuesChangedAllDocuments(
+    const MediaFeatureChange& aChange) {
+  // Inform every content image used in the document that media feature values
+  // have changed.  If the same image is used in multiple places, then we can
+  // end up informing them multiple times.  Theme changes are rare though and we
+  // don't bother trying to ensure we only do this once per image.
+  //
+  // Pull the images out into an array and iterate over them, in case the
+  // image notifications do something that ends up modifying the table.
+  nsTArray<nsCOMPtr<imgIContainer>> images;
+  for (auto iter = mImages.Iter(); !iter.Done(); iter.Next()) {
+    imgIRequest* req = iter.Key();
+    nsCOMPtr<imgIContainer> image;
+    req->GetImage(getter_AddRefs(image));
+    if (!image) {
+      continue;
+    }
+    images.AppendElement(image->Unwrap());
+  }
+  for (imgIContainer* image : images) {
+    image->MediaFeatureValuesChangedAllDocuments(aChange);
+  }
+}
+
 }  // namespace dom
 }  // namespace mozilla

dom/base/ImageTracker.h

@@ -54,6 +54,8 @@ class ImageTracker {
 
   void RequestDiscardAll();
 
+  void MediaFeatureValuesChangedAllDocuments(const MediaFeatureChange&);
+
  private:
   ~ImageTracker();
 

image/VectorImage.cpp

@@ -39,6 +39,7 @@
 #include "nsIDOMEventListener.h"
 #include "SurfaceCache.h"
 #include "mozilla/dom/Document.h"
+#include "mozilla/dom/DocumentInlines.h"
 
 namespace mozilla {
 
@@ -564,7 +565,6 @@ void VectorImage::SendInvalidationNotifications() {
   // we would miss the subsequent invalidations if we didn't send out the
   // notifications indirectly in |InvalidateObservers...|.
 
-  MOZ_ASSERT(mHasPendingInvalidation);
   mHasPendingInvalidation = false;
   SurfaceCache::RemoveImage(ImageKey(this));
 
@@ -1527,5 +1527,35 @@ already_AddRefed<imgIContainer> VectorImage::Unwrap() {
   return self.forget();
 }
 
+void VectorImage::MediaFeatureValuesChangedAllDocuments(
+    const MediaFeatureChange& aChange) {
+  if (!mSVGDocumentWrapper) {
+    return;
+  }
+
+  // Don't bother if the document hasn't loaded yet.
+  if (!mIsFullyLoaded) {
+    return;
+  }
+
+  if (Document* doc = mSVGDocumentWrapper->GetDocument()) {
+    if (nsPresContext* presContext = doc->GetPresContext()) {
+      presContext->MediaFeatureValuesChangedAllDocuments(aChange);
+      // Media feature value changes don't happen in the middle of layout,
+      // so we don't need to call InvalidateObserversOnNextRefreshDriverTick
+      // to invalidate asynchronously.
+      //
+      // Ideally we would not invalidate images if the media feature value
+      // change did not cause any updates to the document, but since non-
+      // animated SVG images do not have their refresh driver ticked, it
+      // is the invalidation (and then the painting) which is what causes
+      // the document to be flushed. Theme and system metrics changes are
+      // rare, though, so it's not a big deal to invalidate even if it
+      // doesn't cause any change.
+      SendInvalidationNotifications();
+    }
+  }
+}
+
 }  // namespace image
 }  // namespace mozilla

image/VectorImage.h

@@ -14,6 +14,8 @@ class nsIRequest;
 class gfxDrawable;
 
 namespace mozilla {
+struct MediaFeatureChange;
+
 namespace image {
 
 struct SVGDrawingParameters;
@@ -32,6 +34,7 @@ class VectorImage final : public ImageResource, public nsIStreamListener {
   // (no public constructor - use ImageFactory)
 
   // Methods inherited from Image
+  void MediaFeatureValuesChangedAllDocuments(const MediaFeatureChange&) final;
   nsresult GetNativeSizes(nsTArray<gfx::IntSize>& aNativeSizes) const override;
   size_t GetNativeSizesLength() const override;
   virtual size_t SizeOfSourceWithComputedFallback(

image/imgIContainer.idl

@@ -33,6 +33,7 @@ class nsIFrame;
 namespace mozilla {
 class TimeStamp;
 class SVGImageContext;
+struct MediaFeatureChange;
 }
 
 namespace mozilla {
@@ -646,6 +647,14 @@ interface imgIContainer : nsISupports
   [noscript, notxpcom] void propagateUseCounters(in Document aDocument);
 
   %{C++
+  /*
+   * Called when media feature values that apply to all documents (such as
+   * those based on system metrics) have changed.  If this image is a type
+   * that can respond to media queries (i.e., an SVG image), this function
+   * is overridden to handle restyling and invalidating the image.
+   */
+  virtual void MediaFeatureValuesChangedAllDocuments(const mozilla::MediaFeatureChange& aChange) {}
+
   /*
    * Get the set of sizes the image can decode to natively.
    */

layout/base/nsPresContext.cpp

@@ -87,6 +87,7 @@
 #include "mozilla/dom/PerformanceTiming.h"
 #include "mozilla/layers/APZThreadUtils.h"
 #include "MobileViewportManager.h"
+#include "mozilla/dom/ImageTracker.h"
 
 // Needed for Start/Stop of Image Animation
 #include "imgIContainer.h"
@@ -1525,7 +1526,15 @@ static bool MediaFeatureValuesChangedAllDocumentsCallback(Document* aDocument,
 
 void nsPresContext::MediaFeatureValuesChangedAllDocuments(
     const MediaFeatureChange& aChange) {
+  // Handle the media feature value change in this document.
   MediaFeatureValuesChanged(aChange);
+
+  // Propagate the media feature value change down to any SVG images the
+  // document is using.
+  mDocument->StyleImageLoader()->MediaFeatureValuesChangedAllDocuments(aChange);
+  mDocument->ImageTracker()->MediaFeatureValuesChangedAllDocuments(aChange);
+
+  // And then into any subdocuments.
   mDocument->EnumerateSubDocuments(
       MediaFeatureValuesChangedAllDocumentsCallback,
       const_cast<MediaFeatureChange*>(&aChange));

layout/style/ImageLoader.cpp

@@ -793,6 +793,30 @@ nsresult ImageLoader::OnLoadComplete(imgIRequest* aRequest) {
   return NS_OK;
 }
 
+void ImageLoader::MediaFeatureValuesChangedAllDocuments(
+    const MediaFeatureChange& aChange) {
+  // Inform every CSS image used in the document that media feature values have
+  // changed.  If the same image is used in multiple places, then we can end up
+  // informing them multiple times.  Theme changes are rare though and we don't
+  // bother trying to ensure we only do this once per image.
+  //
+  // Pull the images out into an array and iterate over them, in case the
+  // image notifications do something that ends up modifying the table.
+  nsTArray<nsCOMPtr<imgIContainer>> images;
+  for (auto iter = mRegisteredImages.Iter(); !iter.Done(); iter.Next()) {
+    imgRequestProxy* req = iter.Data();
+    nsCOMPtr<imgIContainer> image;
+    req->GetImage(getter_AddRefs(image));
+    if (!image) {
+      continue;
+    }
+    images.AppendElement(image->Unwrap());
+  }
+  for (imgIContainer* image : images) {
+    image->MediaFeatureValuesChangedAllDocuments(aChange);
+  }
+}
+
 bool ImageLoader::ImageReflowCallback::ReflowFinished() {
   // Check that the frame is still valid. If it isn't, then onload was
   // unblocked when the frame was removed from the FrameSet in

layout/style/ImageLoader.h

@@ -19,6 +19,7 @@
 #include "imgIRequest.h"
 #include "imgINotificationObserver.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/MediaFeatureChange.h"
 
 class imgIContainer;
 class nsIFrame;
@@ -71,6 +72,12 @@ class ImageLoader final : public imgINotificationObserver {
 
   void SetAnimationMode(uint16_t aMode);
 
+  /**
+   * Called by the document's pres context when media features in all
+   * SVG images must be re-evaluated.
+   */
+  void MediaFeatureValuesChangedAllDocuments(const MediaFeatureChange& aChange);
+
   // The prescontext for this ImageLoader's document. We need it to be passed
   // in because this can be called during presentation destruction after the
   // presshell pointer on the document has been cleared.

layout/xul/nsImageBoxFrame.cpp

@@ -51,6 +51,7 @@
 #include "Units.h"
 #include "mozilla/layers/RenderRootStateManager.h"
 #include "mozilla/layers/WebRenderLayerManager.h"
+#include "mozilla/dom/ImageTracker.h"
 
 #if defined(XP_WIN)
 // Undefine LoadImage to prevent naming conflict with Windows.
@@ -168,6 +169,10 @@ void nsImageBoxFrame::DestroyFrom(nsIFrame* aDestructRoot,
 
     mImageRequest->UnlockImage();
 
+    if (mUseSrcAttr) {
+      PresContext()->Document()->ImageTracker()->Remove(mImageRequest);
+    }
+
     // Release image loader first so that it's refcnt can go to zero
     mImageRequest->CancelAndForgetObserver(NS_ERROR_FAILURE);
   }
@@ -211,6 +216,7 @@ void nsImageBoxFrame::StopAnimation() {
 
 void nsImageBoxFrame::UpdateImage() {
   nsPresContext* presContext = PresContext();
+  Document* doc = presContext->Document();
 
   RefPtr<imgRequestProxy> oldImageRequest = mImageRequest;
 
@@ -218,6 +224,9 @@ void nsImageBoxFrame::UpdateImage() {
     nsLayoutUtils::DeregisterImageRequest(presContext, mImageRequest,
                                           &mRequestRegistered);
     mImageRequest->CancelAndForgetObserver(NS_ERROR_FAILURE);
+    if (mUseSrcAttr) {
+      doc->ImageTracker()->Remove(mImageRequest);
+    }
     mImageRequest = nullptr;
   }
 
@@ -226,31 +235,34 @@ void nsImageBoxFrame::UpdateImage() {
   mContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::src, src);
   mUseSrcAttr = !src.IsEmpty();
   if (mUseSrcAttr) {
-    Document* doc = mContent->GetComposedDoc();
-    if (doc) {
-      nsContentPolicyType contentPolicyType;
-      nsCOMPtr<nsIPrincipal> triggeringPrincipal;
-      uint64_t requestContextID = 0;
-      nsContentUtils::GetContentPolicyTypeForUIImageLoading(
-          mContent, getter_AddRefs(triggeringPrincipal), contentPolicyType,
-          &requestContextID);
+    nsContentPolicyType contentPolicyType;
+    nsCOMPtr<nsIPrincipal> triggeringPrincipal;
+    uint64_t requestContextID = 0;
+    nsContentUtils::GetContentPolicyTypeForUIImageLoading(
+        mContent, getter_AddRefs(triggeringPrincipal), contentPolicyType,
+        &requestContextID);
 
-      nsCOMPtr<nsIURI> uri;
-      nsContentUtils::NewURIWithDocumentCharset(getter_AddRefs(uri), src, doc,
-                                                mContent->GetBaseURI());
-      if (uri) {
-        nsCOMPtr<nsIReferrerInfo> referrerInfo = new ReferrerInfo();
-        referrerInfo->InitWithNode(mContent);
+    nsCOMPtr<nsIURI> uri;
+    nsContentUtils::NewURIWithDocumentCharset(getter_AddRefs(uri), src, doc,
+                                              mContent->GetBaseURI());
+    if (uri) {
+      nsCOMPtr<nsIReferrerInfo> referrerInfo = new ReferrerInfo();
+      referrerInfo->InitWithNode(mContent);
 
-        nsresult rv = nsContentUtils::LoadImage(
-            uri, mContent, doc, triggeringPrincipal, requestContextID,
-            referrerInfo, mListener, mLoadFlags, EmptyString(),
-            getter_AddRefs(mImageRequest), contentPolicyType);
+      nsresult rv = nsContentUtils::LoadImage(
+          uri, mContent, doc, triggeringPrincipal, requestContextID,
+          referrerInfo, mListener, mLoadFlags, EmptyString(),
+          getter_AddRefs(mImageRequest), contentPolicyType);
 
-        if (NS_SUCCEEDED(rv) && mImageRequest) {
-          nsLayoutUtils::RegisterImageRequestIfAnimated(
-              presContext, mImageRequest, &mRequestRegistered);
-        }
+      if (NS_SUCCEEDED(rv) && mImageRequest) {
+        nsLayoutUtils::RegisterImageRequestIfAnimated(
+            presContext, mImageRequest, &mRequestRegistered);
+
+        // Add to the ImageTracker so that we can find it when media
+        // feature values change (e.g. when the system theme changes)
+        // and invalidate the image.  This allows favicons to respond
+        // to these changes.
+        doc->ImageTracker()->Add(mImageRequest);
       }
     }
   } else if (auto* styleRequest = GetRequestFromStyle()) {

layout/xul/tree/nsTreeBodyFrame.cpp

@@ -1926,6 +1926,11 @@ nsresult nsTreeBodyFrame::GetImage(int32_t aRowIndex, nsTreeColumn* aCol,
           imgNotificationObserver, nsIRequest::LOAD_NORMAL, EmptyString(),
           getter_AddRefs(imageRequest));
       NS_ENSURE_SUCCESS(rv, rv);
+
+      // NOTE(heycam): If it's an SVG image, and we need to want the image to
+      // able to respond to media query changes, it needs to be added to the
+      // document's ImageTracker (like nsImageBoxFrame does).  For now, assume
+      // we don't need this.
     }
     listener->UnsuppressInvalidation();