Tracking filter mutation via SVGElementProxy

third_party/WebKit/Source/core/svg/SVGElementProxy.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SVGElementProxy_h
+#define SVGElementProxy_h
+
+#include "core/fetch/DocumentResource.h"
+#include "platform/heap/Handle.h"
+#include "wtf/text/AtomicString.h"
+#include "wtf/text/WTFString.h"
+
+namespace blink {
+
+class Document;
+class SVGElement;
+class SVGResourceClient;
+class TreeScope;
+
+// A proxy to an SVGElement. Allows access to an element with a certain 'id',

[pdr., 2016/10/20 19:03:42]

Do you mind expanding this comment to describe the overall design?

These are the notes I took while reviewing. Maybe something along these lines?
* SVGElements can be referenced using CSS (filter: url(foo.com/bar.svg#baz;)
which may require loading external resources. SVGElementProxy is a persistent
reference to an SVGElement both before and after the SVGElement has been found.
* Filtered content has |CSSURIValue|s which create |SVGElementProxy|s which also
maintain references back to the filtered content via SVGResourceClient.
* SVGElementProxy looks up SVGElements by id (for remote resources, it will
request the SVGDocument and lookup by id there).
* When SVGElementProxy finds an SVGElement, it tells the SVGElement which in
turn keeps a list of |SVGElementProxy|s referencing it.
* These bi-directional references (filtered content <-> SVGElementProxy and
SVGElementProxy <-> SVGFilterElement) are used for invalidation, change
notifications, etc.

[fs, 2016/10/21 11:17:12]

Thanks, I tried to jot something down that I hope included all of the above.

+// and provides its clients with notifications when the reference changes.
+class SVGElementProxy : public GarbageCollectedFinalized<SVGElementProxy>,
+                        private DocumentResourceClient {
+  USING_GARBAGE_COLLECTED_MIXIN(SVGElementProxy);
+  USING_PRE_FINALIZER(SVGElementProxy, detachDocumentClient);
+
+ public:
+  // Create a proxy to an element in the same document. (See also
+  // SVGURLReferenceResolver and the definition of 'local url'.)
+  static SVGElementProxy* create(const AtomicString& id) {
+    return new SVGElementProxy(id);
+  }
+
+  // Create a proxy to an element in a different document (indicated by |url|.)
+  static SVGElementProxy* create(const String& url, const AtomicString& id) {
+    return new SVGElementProxy(url, id);
+  }
+  virtual ~SVGElementProxy();
+
+  void addClient(SVGResourceClient*);
+  void removeClient(SVGResourceClient*);
+
+  // Resolve a potentially external reference.
+  void resolve(Document&);
+
+  // Returns the proxied element, or null if the proxy is invalid.
+  SVGElement* element(TreeScope&);
+
+  // Notify the proxy that the structure of the subtree rooted at the proxied
+  // element has mutated. This should generally only be called via a proxy
+  // tracker.
+  void contentChanged();
+
+  // Notify the proxy that the reference it holds may now be pointing somewhere
+  // else (the id of an element changed, the resource document of the proxy
+  // finished loading, the element was removed from the document tree etc.)
+  // This should generally only be called via a proxy tracker.
+  void referenceChanged();
+
+  const AtomicString& id() const { return m_id; }
+
+  // The current "generation" of this proxy. This is essentially a sequence
+  // number (or counter) incremented with each change to the proxy reference.

[pdr., 2016/10/20 19:03:42]

Nit: "reference" was a little confusing to me when reviewing this because the
proxy both references and is referenced. I don't have a great suggestion for
making this clearer though. Maybe "proxied element" (and
proxiedElementChanged(), above)?

[fs, 2016/10/21 11:17:12]

I tried to replace "reference" with "proxied element" (or add it for
disambiguation) here and in a few other places. Also renamed the method (and the
corresponding callback on SVGResourceClient.)

+  unsigned generation() const { return m_generation; }
+
+  DECLARE_VIRTUAL_TRACE();
+
+ private:
+  explicit SVGElementProxy(const AtomicString&);
+  SVGElementProxy(const String&, const AtomicString&);
+
+  void detachDocumentClient();
+  // ResourceClient implementation. Used when referencing an external resource
+  // document.
+  void notifyFinished(Resource*) override;
+  String debugName() const override { return "SVGElementProxy"; }
+
+  void incrementGeneration() { m_generation++; }
+  TreeScope* treeScopeForLookup(TreeScope&) const;
+
+  HeapHashSet<Member<SVGResourceClient>> m_clients;
+  Member<DocumentResource> m_document;
+  AtomicString m_id;
+  // URL for resolving references to external resource documents. Contains an
+  // absolute URL to the resource to load. Cleared when a load has been
+  // initiated. Ignored if m_isLocal is true.
+  String m_url;
+  unsigned m_generation;
+  bool m_isLocal;
+};
+
+// Collection of SVGElementProxys. This is hosted by elements that can be
+// subject to proxies (currently only SVGFilterElement), and is mainly a helper
+// for dealing with the many-to-one structure of SVGElementProxy.
+class SVGElementProxySet : public GarbageCollected<SVGElementProxySet> {
+ public:
+  void add(SVGElementProxy&);
+  bool isEmpty() const;
+
+  // Notify proxy clients that the (content of the) proxied element has
+  // changed.
+  void notifyContentChanged();
+
+  // Notify proxy clients that the reference to the is no longer valid. This
+  // also clears references to the proxied element.
+  void invalidateProxies();
+
+  DECLARE_TRACE();
+
+ private:
+  using ProxySet = HeapHashSet<WeakMember<SVGElementProxy>>;
+  ProxySet m_elementProxies;
+};
+
+}  // namespace blink
+
+#endif  // SVGElementProxy_h