Adds SVGImageElement as a CanvasImageSource

third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h

+/*
+ *
+ * Copyright (C) 2017 Google Inc. All rights reserved.

[fs, 2017/03/06 22:46:15]

Use short copyright.
(https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...)

[fserb, 2017/03/07 20:44:02]

done.

+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public License
+ * along with this library; see the file COPYING.LIB.  If not, write to
+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ *
+ */
+
+#ifndef CanvasHTMLSVGImage_h
+#define CanvasHTMLSVGImage_h
+
+#include "bindings/core/v8/ScriptWrappable.h"

[fs, 2017/03/06 22:46:15]

What's ScriptWrappable here

[fserb, 2017/03/07 20:44:02]

sry, my setup used to lint about those and it doesn't for Blink. :/

+#include "core/CoreExport.h"
+#include "core/html/canvas/CanvasImageSource.h"
+#include "core/loader/ImageLoader.h"
+#include "platform/heap/Visitor.h"

[fs, 2017/03/06 22:46:15]

Not used? (In general it seems very little besides CanvasImageSource.h would
need to be included here, so maybe double-check and clean up as needed here.)

[fserb, 2017/03/07 20:44:02]

done.

+
+namespace blink {
+
+class CORE_EXPORT CanvasHTMLSVGImage : public CanvasImageSource {

[fs, 2017/03/06 22:46:15]

The ...HTMLSVGImage suffix feels a bit weird. Maybe CanvasImageElementSource or
something instead?

[fserb, 2017/03/07 20:44:02]

done.

+ public:
+  virtual ImageResourceContent* cachedImage() const = 0;

[fs, 2017/03/06 22:46:15]

It's a bit unfortunate that this ended up as virtual, seeing as though it's
called pretty often. If this remains, try to make it so it's not called too much
per method. (HTML/SVGImageElement are both final, so hopefully it's
devirtualized there.)
That being said, I still think using the ImageLoader would be better - it
already has a Element reference, so the element() method here wouldn't be
needed, and imageLoaded() can be trivially implemented from that as well. The
only problem seems to be that both HTMLImageElement and SVGImageElement have
imageLoader() methods that return their derived ImageLoader types, so a
different name might be needed to avoid clashing.

[fserb, 2017/03/07 20:44:02]

The ImageLoader is a great idea. I moved that to be the only virtual and moved
the other helper functions private on the class.
It's still virtual, but as you said, they are both final, so maybe it's ok to
leave like that?

[fs, 2017/03/07 22:01:24]

Yeah, let's hope so. (Feel free to verify that it indeed is.)

+  virtual bool imageLoaded() const = 0;
+  virtual const Element* element() const = 0;
+
+  PassRefPtr<Image> getSourceImageForCanvas(SourceImageStatus*,
+                                            AccelerationHint,
+                                            SnapshotReason,
+                                            const FloatSize&) const override;
+
+  bool wouldTaintOrigin(
+      SecurityOrigin* destinationSecurityOrigin) const override;
+
+  FloatSize elementSize(const FloatSize& defaultObjectSize) const override;
+  FloatSize defaultDestinationSize(
+      const FloatSize& defaultObjectSize) const override;
+
+  bool isAccelerated() const override;
+
+  int sourceWidth() override;
+  int sourceHeight() override;
+
+  bool isSVGSource() const override;
+
+  bool isOpaque() const override;
+
+  const KURL& sourceURL() const override;
+
+ private:

[fs, 2017/03/06 22:46:15]

Drop?

[fserb, 2017/03/07 20:44:02]

used now.

+};
+
+}  // namespace blink
+
+#endif  // CanvasHTMLSVGImage_h