Revert "[skjson] Size-constrained input API"

This reverts commit fc792b8718cc30e9da62c9559b23c1baac3166bb.

Reason for revert: New ASAN, Android failures.

Original change's description:
> [skjson] Size-constrained input API
> 
> Pass an explicit input size instead of requiring a C string.
> 
> Thanks to mtklein's clever trick, this has no measurable perf impact.
> 
> Change-Id: I64f210a9f653a78b05ab6b58fa34479504aa35ff
> Reviewed-on: https://skia-review.googlesource.com/134940
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: Florin Malita <fmalita@chromium.org>

TBR=mtklein@google.com,fmalita@chromium.org

Change-Id: Ic0b52398b1ce6f64c781debb858829cb64bbae58
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/135022
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>

5 files changed, 41 insertions, 50 deletions

diff --git a/modules/skjson/include/SkJSON.h b/modules/skjson/include/SkJSON.h
index 1b46c4a100..99da29de72 100644
--- a/modules/skjson/include/SkJSON.h
+++ b/modules/skjson/include/SkJSON.h
@@ -326,7 +326,7 @@ private:
 
 class DOM final : public SkNoncopyable {
 public:
-    DOM(const char*, size_t);
+    explicit DOM(const char*);
 
     const Value& root() const { return fRoot; }
 
diff --git a/modules/skjson/src/FuzzSkJSON.cpp b/modules/skjson/src/FuzzSkJSON.cpp
index 2e971ce249..ce33cc1299 100644
--- a/modules/skjson/src/FuzzSkJSON.cpp
+++ b/modules/skjson/src/FuzzSkJSON.cpp
@@ -5,12 +5,20 @@
  * found in the LICENSE file.
  */
 
+#include "SkAutoMalloc.h"
 #include "SkData.h"
 #include "SkJSON.h"
 #include "SkStream.h"
 
 void FuzzSkJSON(sk_sp<SkData> bytes) {
-    skjson::DOM dom(static_cast<const char*>(bytes->data()), bytes->size());
+    // TODO: add a size + len skjson::DOM factory?
+    SkAutoMalloc data(bytes->size() + 1);
+    auto* c_str = static_cast<char*>(data.get());
+
+    memcpy(c_str, bytes->data(), bytes->size());
+    c_str[bytes->size()] = '\0';
+
+    skjson::DOM dom(c_str);
     SkDynamicMemoryWStream wstream;
     dom.write(&wstream);
 }
diff --git a/modules/skjson/src/SkJSON.cpp b/modules/skjson/src/SkJSON.cpp
index 64f1302a7e..bf176c5c5c 100644
--- a/modules/skjson/src/SkJSON.cpp
+++ b/modules/skjson/src/SkJSON.cpp
@@ -11,12 +11,12 @@
 #include "SkString.h"
 
 #include <cmath>
-#include <tuple>
 #include <vector>
 
 namespace skjson {
 
-// #define SK_JSON_REPORT_ERRORS
+//#define SK_JSON_REPORT_ERRORS
+
 
 static_assert( sizeof(Value) == 8, "");
 static_assert(alignof(Value) == 8, "");
@@ -236,21 +236,7 @@ public:
         fScopeStack.reserve(kScopeStackReserve);
     }
 
-    const Value parse(const char* p, size_t size) {
-        if (!size) {
-            this->error(NullValue(), p, "invalid empty input");
-        }
-
-        const char* p_stop = p + size - 1;
-
-        // We're only checking for end-of-stream on object/array close('}',']'),
-        // so we must trim any whitespace from the buffer tail.
-        while (p_stop > p && is_ws(*p_stop)) --p_stop;
-
-        if (*p_stop != '}' && *p_stop != ']') {
-            return this->error(NullValue(), p_stop, "invalid top-level value");
-        }
-
+    const Value parse(const char* p) {
         p = skip_ws(p);
 
         switch (*p) {
@@ -354,17 +340,17 @@ public:
     pop_common:
         SkASSERT(*p == '}' || *p == ']');
 
+        ++p;
+
         if (fScopeStack.empty()) {
             SkASSERT(fValueStack.size() == 1);
 
-            // Success condition: parsed the top level element and reached the stop token.
-            return p == p_stop
+            // Stop condition: parsed the top level element and there is no trailing garbage.
+            return *skip_ws(p) == '\0'
                 ? fValueStack.front()
-                : this->error(NullValue(), p + 1, "trailing root garbage");
+                : this->error(NullValue(), p, "trailing root garbage");
         }
 
-        ++p;
-
         goto match_post_value;
 
     match_array:
@@ -391,20 +377,19 @@ public:
         return NullValue();
     }
 
-    std::tuple<const char*, const SkString> getError() const {
-        return std::make_tuple(fErrorToken, fErrorMessage);
+    const SkString& getError() const {
+        return fError;
     }
 
 private:
-    SkArenaAlloc&         fAlloc;
+    SkArenaAlloc&                 fAlloc;
 
     static constexpr size_t kValueStackReserve = 256;
     static constexpr size_t kScopeStackReserve = 128;
     std::vector<Value   > fValueStack;
     std::vector<intptr_t> fScopeStack;
 
-    const char*           fErrorToken = nullptr;
-    SkString              fErrorMessage;
+    SkString             fError;
 
     template <typename VectorT>
     void popScopeAsVec(size_t scope_start) {
@@ -505,8 +490,9 @@ private:
     template <typename T>
     T error(T&& ret_val, const char* p, const char* msg) {
 #if defined(SK_JSON_REPORT_ERRORS)
-        fErrorToken = p;
-        fErrorMessage.set(msg);
+        static constexpr size_t kMaxContext = 128;
+        fError = SkStringPrintf("%s: >", msg);
+        fError.append(p, std::min(strlen(p), kMaxContext));
 #endif
         return ret_val;
     }
@@ -732,11 +718,11 @@ SkString Value::toString() const {
 
 static constexpr size_t kMinChunkSize = 4096;
 
-DOM::DOM(const char* data, size_t size)
+DOM::DOM(const char* c_str)
     : fAlloc(kMinChunkSize) {
     DOMParser parser(fAlloc);
 
-    fRoot = parser.parse(data, size);
+    fRoot = parser.parse(c_str);
 }
 
 void DOM::write(SkWStream* stream) const {
diff --git a/modules/skjson/src/SkJSONBench.cpp b/modules/skjson/src/SkJSONBench.cpp
index b9c3664306..4be3b972ab 100644
--- a/modules/skjson/src/SkJSONBench.cpp
+++ b/modules/skjson/src/SkJSONBench.cpp
@@ -25,21 +25,27 @@ protected:
     bool isSuitableFor(Backend backend) override { return backend == kNonRendering_Backend; }
 
     void onPerCanvasPreDraw(SkCanvas*) override {
-        fData = SkData::MakeFromFileName(kBenchFile);
-        if (!fData) {
+        if (auto stream = SkStream::MakeFromFile(kBenchFile)) {
+            SkASSERT(stream->hasLength());
+            fCStringData = SkData::MakeUninitialized(stream->getLength() + 1);
+            auto* data8 = reinterpret_cast<uint8_t*>(fCStringData->writable_data());
+            SkAssertResult(stream->read(data8, stream->getLength()) == stream->getLength());
+            data8[stream->getLength()] = '\0';
+
+        } else {
             SkDebugf("!! Could not open bench file: %s\n", kBenchFile);
         }
     }
 
     void onPerCanvasPostDraw(SkCanvas*) override {
-        fData = nullptr;
+        fCStringData = nullptr;
     }
 
     void onDraw(int loops, SkCanvas*) override {
-        if (!fData) return;
+        if (!fCStringData) return;
 
         for (int i = 0; i < loops; i++) {
-            skjson::DOM dom(static_cast<const char*>(fData->data()), fData->size());
+            skjson::DOM dom(static_cast<const char*>(fCStringData->data()));
             if (dom.root().is<skjson::NullValue>()) {
                 SkDebugf("!! Parsing failed.\n");
                 return;
@@ -48,7 +54,7 @@ protected:
     }
 
 private:
-    sk_sp<SkData> fData;
+    sk_sp<SkData> fCStringData;
 
     using INHERITED = Benchmark;
 };
diff --git a/modules/skjson/src/SkJSONTest.cpp b/modules/skjson/src/SkJSONTest.cpp
index 91ae497cf0..6d4338cbbb 100644
--- a/modules/skjson/src/SkJSONTest.cpp
+++ b/modules/skjson/src/SkJSONTest.cpp
@@ -22,14 +22,8 @@ DEF_TEST(SkJSON_Parse, reporter) {
         { ""     , nullptr },
         { "["    , nullptr },
         { "]"    , nullptr },
-        { "[[]"  , nullptr },
-        { "[]]"  , nullptr },
-        { "[]f"  , nullptr },
         { "{"    , nullptr },
         { "}"    , nullptr },
-        { "{{}"  , nullptr },
-        { "{}}"  , nullptr },
-        { "{}f"  , nullptr },
         { "{]"   , nullptr },
         { "[}"   , nullptr },
         { "1"    , nullptr },
@@ -57,9 +51,7 @@ DEF_TEST(SkJSON_Parse, reporter) {
         { "{ \"k\" : null \"k\" : 1 }", nullptr },
 
 
-        { "[]"                           , "[]" },
         { " \n\r\t [ \n\r\t ] \n\r\t "   , "[]" },
-        { "[[]]"                         , "[[]]" },
         { "[ null ]"                     , "[null]" },
         { "[ true ]"                     , "[true]" },
         { "[ false ]"                    , "[false]" },
@@ -74,7 +66,6 @@ DEF_TEST(SkJSON_Parse, reporter) {
         { "[ \"123456789\" ]"            , "[\"123456789\"]" },
         { "[ null , true, false,0,12.8 ]", "[null,true,false,0,12.8]" },
 
-        { "{}"                          , "{}" },
         { " \n\r\t { \n\r\t } \n\r\t "  , "{}" },
         { "{ \"k\" : null }"            , "{\"k\":null}" },
         { "{ \"k1\" : null, \"k2 \":0 }", "{\"k1\":null,\"k2 \":0}" },
@@ -98,7 +89,7 @@ DEF_TEST(SkJSON_Parse, reporter) {
     };
 
     for (const auto& tst : g_tests) {
-        DOM dom(tst.in, strlen(tst.in));
+        DOM dom(tst.in);
         const auto success = !dom.root().is<NullValue>();
         REPORTER_ASSERT(reporter, success == (tst.out != nullptr));
         if (!success) continue;
@@ -162,7 +153,7 @@ DEF_TEST(SkJSON_DOM_visit, reporter) {
         \"k8\": { \"kk1\": 2, \"kk2\": false, \"kk1\": \"baz\" } \n\
     }";
 
-    DOM dom(json, strlen(json));
+    DOM dom(json);
 
     const auto& jroot = dom.root().as<ObjectValue>();
     REPORTER_ASSERT(reporter, jroot.is<ObjectValue>());