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',
+// 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 element was removed from the
+  // document tree etc.) This should generally only be called via a proxy
+  // tracker.

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

This also seems to be called from notifyFinished

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

That's kind of an implementation detail that ought to be transparent to users of
this interface. Technically speaking it can also be considered covered by "the
id of an element changed". I'll add it to the list of events though.

+  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 03:11:05]

Are there any security issues here if a mean user spams proxies until this
overflows?

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

What you can achieve by that is to make two operations compare equal when they
should not. That would make the client(s) reference the wrong proxy/element (==
failed invalidations), but nothing more severe that that I can think of.

+  unsigned generation() const { return m_generation; }
+
+  DECLARE_VIRTUAL_TRACE();
+
+ private:
+  explicit SVGElementProxy(const AtomicString&);
+  SVGElementProxy(const String&, const AtomicString&);
+
+  void detachDocumentClient();
+  void notifyFinished(Resource*) override;

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

Can you add a comment here about how this is called when the referenced document
loads?

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

Done.

+  String debugName() const override { return "SVGElementProxy"; }
+
+  void nextGeneration() { m_generation++; }

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

Bikeshed nit: incrementCacheGeneration?

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

Made it incrementGeneration (there's nothing "cache" here really.)

+  TreeScope* treeScopeForLookup(TreeScope&) const;
+
+  HeapHashSet<Member<SVGResourceClient>> m_clients;
+  Member<DocumentResource> m_document;
+  AtomicString m_id;
+  String m_url;

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

Can you add a comment about how this will be cleared after the request
completes?

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

Done.

+  unsigned m_generation;
+  bool m_isLocal;
+};
+
+// Tracker 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 SVGElementProxyTracker : public GarbageCollected<SVGElementProxyTracker> {

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

This name confused me for a bit because I thought it was more than just a
set+helper. WDYT of renaming to just SVGElementProxySet?

(further suggestions below, none required).

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

Renamed to SVGElementProxySet.

+ public:
+  void attachElementProxy(SVGElementProxy&);

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

Maybe just "add(SVGElementProxy&)"?

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

Done.

+  bool isEmpty() const;
+
+  // Notify proxy clients that the (content of the) proxied element has
+  // changed.
+  void invalidateProxyClients();

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

Maybe just "notifyThatContentChanged"?

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

Done (minus "That".)

+
+  // 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:
+  HeapHashSet<WeakMember<SVGElementProxy>> m_elementProxies;
+};
+
+}  // namespace blink
+
+#endif  // SVGElementProxy_h