Tracking filter mutation via SVGElementProxy

third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp

Index: third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp
diff --git a/third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp b/third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp
index 97bbc8779773203c9f9f874d67edfb4deea98519..e1fb264d04816e27a7d0515bf1cee08bb5c293c8 100644
--- a/third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp
+++ b/third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp
@@ -30,7 +30,6 @@
 #include "core/css/CSSURIValue.h"
 #include "core/dom/Document.h"
 #include "core/fetch/ResourceFetcher.h"
-#include "core/layout/svg/ReferenceFilterBuilder.h"
 #include "core/style/ComputedStyle.h"
 #include "core/style/ContentData.h"
 #include "core/style/CursorData.h"
@@ -42,6 +41,7 @@
 #include "core/style/StyleImage.h"
 #include "core/style/StyleInvalidImage.h"
 #include "core/style/StylePendingImage.h"
+#include "core/svg/SVGElementProxy.h"
 
 namespace blink {
 
@@ -108,36 +108,23 @@ StyleImage* ElementStyleResources::cursorOrPendingFromValue(
   return value.cachedImage(m_deviceScaleFactor);
 }
 
-void ElementStyleResources::addPendingSVGDocument(
-    FilterOperation* filterOperation,
-    const CSSURIValue* cssUriValue) {
-  m_pendingSVGDocuments.set(filterOperation, cssUriValue);
+SVGElementProxy* ElementStyleResources::cachedOrPendingFromValue(
+    const CSSURIValue& value) {
+  return value.createElementProxy(*m_document);
 }
 
 void ElementStyleResources::loadPendingSVGDocuments(
     ComputedStyle* computedStyle) {
-  if (!computedStyle->hasFilter() || m_pendingSVGDocuments.isEmpty())
+  if (!computedStyle->hasFilter())

[pdr., 2016/10/20 03:11:04]

Looking at the comment about images in loadPendingImages, we can request a bunch
of images but only end up needing one. For example:
<style>
   div { background-image: url(a.png); }
   div { background-image: url(b.png); }
   div { background-image: none; }
</style>
<div></div>

My understanding of the new code is that we'd create a bunch of
SVGElementProxies but only resolve the used ones?

[fs, 2016/10/20 11:28:00]

That is correct, yes. The option would be to create some form of placeholder
(like StylePendingImage) and return that, but seeing that an (unresolved) proxy
isn't too heavy I've opted against doing that.

(It might also be worth noting that a CSSURIValue will only create one proxy and
cache it, so createElementProxy will always return the same object. (For now,
see first parenthesis below.)

     return;
-
   FilterOperations::FilterOperationVector& filterOperations =
       computedStyle->mutableFilter().operations();
-  for (unsigned i = 0; i < filterOperations.size(); ++i) {
-    FilterOperation* filterOperation = filterOperations.at(i);
-    if (filterOperation->type() == FilterOperation::REFERENCE) {
-      ReferenceFilterOperation* referenceFilter =
-          toReferenceFilterOperation(filterOperation);
-
-      const CSSURIValue* value = m_pendingSVGDocuments.get(referenceFilter);
-      if (!value)
-        continue;
-      DocumentResource* resource = value->load(*m_document);
-      if (!resource)
-        continue;
-
-      // Stash the DocumentResource on the reference filter.
-      ReferenceFilterBuilder::setDocumentResourceReference(
-          referenceFilter, new DocumentResourceReference(resource));
-    }
+  for (auto& filterOperation : filterOperations) {
+    if (filterOperation->type() != FilterOperation::REFERENCE)
+      continue;
+    ReferenceFilterOperation& referenceOperation =
+        toReferenceFilterOperation(*filterOperation);
+    referenceOperation.elementProxy().resolve(*m_document);

[pdr., 2016/10/20 03:11:04]

If the same url is requested multiple times by different references, will we
create a request for each?

[fs, 2016/10/20 11:27:59]

No. After the proxy is resolved the first time, this is a no-op. (Barring a fix
for crbug.com/405315, which would require a reset of that state.) If multiple
CSSURIValues reference the same resource (non-local document), the MemoryCache
would provide the same Resource. (Excepting fetching restrictions which prevent
re-use.)

   }
 }