Fix DM race in SVGSrc

The SVG source currently attempts to build the DOM lazily, in response
to draw() or size() calls.  But apprently DM may call these
concurrently.

Build the DOM in the constructor instead, to avoid races (and the need
for synchronization).

R=robertphillips@google.com,mtklein@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2482123003

Review-Url: https://codereview.chromium.org/2482123003

2 files changed, 33 insertions, 38 deletions

diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 8bd80e3809..2c5ee16e75 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -1098,46 +1098,43 @@ static const SkSize kDefaultSVGSize = SkSize::Make(1000, 1000);
 // Used to force-scale tiny fixed-size images.
 static const SkSize kMinimumSVGSize = SkSize::Make(128, 128);
 
-SVGSrc::SVGSrc(Path path) : fPath(path), fScale(1) {}
-
-Error SVGSrc::ensureDom() const {
-    if (!fDom) {
-        SkFILEStream stream(fPath.c_str());
-        if (!stream.isValid()) {
-            return SkStringPrintf("Unable to open file: %s", fPath.c_str());
-        }
-        fDom = SkSVGDOM::MakeFromStream(stream);
-        if (!fDom) {
-            return SkStringPrintf("Unable to parse file: %s", fPath.c_str());
-        }
-
-        const SkSize& sz = fDom->containerSize();
-        if (sz.isEmpty()) {
-            // no intrinsic size
-            fDom->setContainerSize(kDefaultSVGSize);
-        } else {
-            fScale = SkTMax(1.f, SkTMax(kMinimumSVGSize.width()  / sz.width(),
-                                        kMinimumSVGSize.height() / sz.height()));
-        }
-    }
-
-    return "";
+SVGSrc::SVGSrc(Path path)
+    : fName(SkOSPath::Basename(path.c_str()))
+    , fScale(1) {
+
+  SkFILEStream stream(path.c_str());
+  if (!stream.isValid()) {
+      return;
+  }
+  fDom = SkSVGDOM::MakeFromStream(stream);
+  if (!fDom) {
+      return;
+  }
+
+  const SkSize& sz = fDom->containerSize();
+  if (sz.isEmpty()) {
+      // no intrinsic size
+      fDom->setContainerSize(kDefaultSVGSize);
+  } else {
+      fScale = SkTMax(1.f, SkTMax(kMinimumSVGSize.width()  / sz.width(),
+                                  kMinimumSVGSize.height() / sz.height()));
+  }
 }
 
 Error SVGSrc::draw(SkCanvas* canvas) const {
-    Error err = this->ensureDom();
-    if (err.isEmpty()) {
-        SkAutoCanvasRestore acr(canvas, true);
-        canvas->scale(fScale, fScale);
-        fDom->render(canvas);
+    if (!fDom) {
+        return SkStringPrintf("Unable to parse file: %s", fName.c_str());
     }
 
-    return err;
+    SkAutoCanvasRestore acr(canvas, true);
+    canvas->scale(fScale, fScale);
+    fDom->render(canvas);
+
+    return "";
 }
 
 SkISize SVGSrc::size() const {
-    Error err = this->ensureDom();
-    if (!err.isEmpty()) {
+    if (!fDom) {
         return SkISize::Make(0, 0);
     }
 
@@ -1145,7 +1142,7 @@ SkISize SVGSrc::size() const {
                         fDom->containerSize().height() * fScale).toRound();
 }
 
-Name SVGSrc::name() const { return SkOSPath::Basename(fPath.c_str()); }
+Name SVGSrc::name() const { return fName; }
 
 bool SVGSrc::veto(SinkFlags flags) const {
     // No need to test to non-(raster||gpu) or indirect backends.
diff --git a/dm/DMSrcSink.h b/dm/DMSrcSink.h
index bb60115051..cac8848307 100644
--- a/dm/DMSrcSink.h
+++ b/dm/DMSrcSink.h
@@ -261,11 +261,9 @@ public:
     bool veto(SinkFlags) const override;
 
 private:
-    Error ensureDom() const;
-
-    Path                    fPath;
-    mutable sk_sp<SkSVGDOM> fDom;
-    mutable SkScalar        fScale;
+    Name            fName;
+    sk_sp<SkSVGDOM> fDom;
+    SkScalar        fScale;
 
     typedef Src INHERITED;
 };