Add SK_LENIENT_SKSL_DESERIALIZATION, enabled in Debugger

When deserializing an SkSL shader, it's possible to encounter code that
doesn't compile (particularly from the last year, when syntax tweaks
were being made). By default, we still treat this as an error and fail
to load the SKP. In the SKP debugger, though, we add a lenient mode that
permits these failures. If any of the children were SkShaders, we
instead return that. If all else fails, we just return nullptr, which
will cause the triggering draw to use a solid paint-color, but at least
allow the SKP to be loaded and viewed.

For color-filters and blenders, we allow malformed SkSL, but always
return nullptr.

The bug that prompted this change involved Android's overscroll stretch
shader, where this approach works particularly well (it turns the
stretch into a simple pass-through of the underlying content).

Change-Id: I756c694739d31b11efa1b82c126f34440a7de66a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515543
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

include/effects/SkRuntimeEffect.h

@@ -184,6 +184,9 @@ public:
         ChildPtr(sk_sp<SkColorFilter> cf) : fChild(std::move(cf)) {}
         ChildPtr(sk_sp<SkBlender> b) : fChild(std::move(b)) {}
 
+        // Asserts that the flattenable is either null, or one of the legal derived types
+        ChildPtr(sk_sp<SkFlattenable> f);
+
         std::optional<ChildType> type() const;
 
         SkShader* shader() const;

src/core/SkReadBuffer.cpp

@@ -393,7 +393,7 @@ sk_sp<SkTypeface> SkReadBuffer::readTypeface() {
     }
 }
 
-SkFlattenable* SkReadBuffer::readFlattenable(SkFlattenable::Type ft) {
+SkFlattenable* SkReadBuffer::readRawFlattenable() {
     SkFlattenable::Factory factory = nullptr;
 
     if (fFactoryCount > 0) {
@@ -446,10 +446,6 @@ SkFlattenable* SkReadBuffer::readFlattenable(SkFlattenable::Type ft) {
             this->validate(false);
             return nullptr;
         }
-        if (obj && obj->getFlattenableType() != ft) {
-            this->validate(false);
-            return nullptr;
-        }
     } else {
         // we must skip the remaining data
         this->skip(sizeRecorded);
@@ -460,6 +456,15 @@ SkFlattenable* SkReadBuffer::readFlattenable(SkFlattenable::Type ft) {
     return obj.release();
 }
 
+SkFlattenable* SkReadBuffer::readFlattenable(SkFlattenable::Type ft) {
+    SkFlattenable* obj = this->readRawFlattenable();
+    if (obj && obj->getFlattenableType() != ft) {
+        this->validate(false);
+        return nullptr;
+    }
+    return obj;
+}
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 int32_t SkReadBuffer::checkInt(int32_t min, int32_t max) {

src/core/SkReadBuffer.h

@@ -112,6 +112,7 @@ public:
         return SkPaintPriv::Unflatten(*this);
     }
 
+    SkFlattenable* readRawFlattenable();
     SkFlattenable* readFlattenable(SkFlattenable::Type);
     template <typename T> sk_sp<T> readFlattenable() {
         return sk_sp<T>((T*)this->readFlattenable(T::GetFlattenableType()));

src/core/SkRuntimeEffect.cpp

@@ -49,10 +49,32 @@
 
 #include <algorithm>
 
+#if defined(SK_BUILD_FOR_DEBUGGER)
+    #define SK_LENIENT_SKSL_DESERIALIZATION 1
+#else
+    #define SK_LENIENT_SKSL_DESERIALIZATION 0
+#endif
+
 using ChildType = SkRuntimeEffect::ChildType;
 
 #ifdef SK_ENABLE_SKSL
 
+static bool flattenable_is_valid_as_child(const SkFlattenable* f) {
+    if (!f) { return true; }
+    switch (f->getFlattenableType()) {
+        case SkFlattenable::kSkShader_Type:
+        case SkFlattenable::kSkColorFilter_Type:
+        case SkFlattenable::kSkBlender_Type:
+            return true;
+        default:
+            return false;
+    }
+}
+
+SkRuntimeEffect::ChildPtr::ChildPtr(sk_sp<SkFlattenable> f) : fChild(std::move(f)) {
+    SkASSERT(flattenable_is_valid_as_child(fChild.get()));
+}
+
 static sk_sp<SkSL::SkVMDebugTrace> make_skvm_debug_trace(SkRuntimeEffect* effect,
                                                          const SkIPoint& coord) {
     auto debugTrace = sk_make_sp<SkSL::SkVMDebugTrace>();
@@ -114,27 +136,41 @@ static bool verify_child_effects(const std::vector<SkRuntimeEffect::Child>& refl
     return true;
 }
 
+/**
+ * If `effect` is specified, then the number and type of child objects are validated against the
+ * children() of `effect`. If it's nullptr, this is skipped, allowing deserialization of children,
+ * even when the effect could not be constructed (ie, due to malformed SkSL).
+ */
 static bool read_child_effects(SkReadBuffer& buffer,
                                const SkRuntimeEffect* effect,
                                SkTArray<SkRuntimeEffect::ChildPtr>* children) {
     size_t childCount = buffer.read32();
-    if (!buffer.validate(childCount == effect->children().size())) {
+    if (effect && !buffer.validate(childCount == effect->children().size())) {
         return false;
     }
 
     children->reset();
     children->reserve_back(childCount);
 
-    for (const auto& child : effect->children()) {
-        if (child.type == ChildType::kShader) {
-            children->emplace_back(buffer.readShader());
-        } else if (child.type == ChildType::kColorFilter) {
-            children->emplace_back(buffer.readColorFilter());
-        } else if (child.type == ChildType::kBlender) {
-            children->emplace_back(buffer.readBlender());
-        } else {
+    for (size_t i = 0; i < childCount; i++) {
+        sk_sp<SkFlattenable> obj(buffer.readRawFlattenable());
+        if (!flattenable_is_valid_as_child(obj.get())) {
+            buffer.validate(false);
             return false;
         }
+        children->push_back(std::move(obj));
+    }
+
+    // If we are validating against an effect, make sure any (non-null) children are the right type
+    if (effect) {
+        auto childInfo = effect->children();
+        SkASSERT(childInfo.size() == children->size());
+        for (size_t i = 0; i < childCount; i++) {
+            std::optional<ChildType> ct = (*children)[i].type();
+            if (ct.has_value() && (*ct) != childInfo[i].type) {
+                buffer.validate(false);
+            }
+        }
     }
 
     return buffer.isValid();
@@ -1084,15 +1120,24 @@ sk_sp<SkFlattenable> SkRuntimeColorFilter::CreateProc(SkReadBuffer& buffer) {
     sk_sp<SkData> uniforms = buffer.readByteArrayAsData();
 
     auto effect = SkMakeCachedRuntimeEffect(SkRuntimeEffect::MakeForColorFilter, std::move(sksl));
+#if !SK_LENIENT_SKSL_DESERIALIZATION
     if (!buffer.validate(effect != nullptr)) {
         return nullptr;
     }
+#endif
 
     SkSTArray<4, SkRuntimeEffect::ChildPtr> children;
     if (!read_child_effects(buffer, effect.get(), &children)) {
         return nullptr;
     }
 
+#if SK_LENIENT_SKSL_DESERIALIZATION
+    if (!effect) {
+        SkDebugf("Serialized SkSL failed to compile. Ignoring/dropping SkSL color filter.\n");
+        return nullptr;
+    }
+#endif
+
     return effect->makeColorFilter(std::move(uniforms), SkMakeSpan(children));
 }
 
@@ -1219,15 +1264,33 @@ sk_sp<SkFlattenable> SkRTShader::CreateProc(SkReadBuffer& buffer) {
     }
 
     auto effect = SkMakeCachedRuntimeEffect(SkRuntimeEffect::MakeForShader, std::move(sksl));
+#if !SK_LENIENT_SKSL_DESERIALIZATION
     if (!buffer.validate(effect != nullptr)) {
         return nullptr;
     }
+#endif
 
     SkSTArray<4, SkRuntimeEffect::ChildPtr> children;
     if (!read_child_effects(buffer, effect.get(), &children)) {
         return nullptr;
     }
 
+#if SK_LENIENT_SKSL_DESERIALIZATION
+    if (!effect) {
+        // If any children were SkShaders, return the first one. This is a reasonable fallback.
+        for (int i = 0; i < children.count(); i++) {
+            if (children[i].shader()) {
+                SkDebugf("Serialized SkSL failed to compile. Replacing shader with child %d.\n", i);
+                return sk_ref_sp(children[i].shader());
+            }
+        }
+
+        // We don't know what to do, so just return nullptr (but *don't* poison the buffer).
+        SkDebugf("Serialized SkSL failed to compile. Ignoring/dropping SkSL shader.\n");
+        return nullptr;
+    }
+#endif
+
     return effect->makeShader(std::move(uniforms), SkMakeSpan(children), localMPtr);
 }
 
@@ -1304,15 +1367,24 @@ sk_sp<SkFlattenable> SkRuntimeBlender::CreateProc(SkReadBuffer& buffer) {
     sk_sp<SkData> uniforms = buffer.readByteArrayAsData();
 
     auto effect = SkMakeCachedRuntimeEffect(SkRuntimeEffect::MakeForBlender, std::move(sksl));
+#if !SK_LENIENT_SKSL_DESERIALIZATION
     if (!buffer.validate(effect != nullptr)) {
         return nullptr;
     }
+#endif
 
     SkSTArray<4, SkRuntimeEffect::ChildPtr> children;
     if (!read_child_effects(buffer, effect.get(), &children)) {
         return nullptr;
     }
 
+#if SK_LENIENT_SKSL_DESERIALIZATION
+    if (!effect) {
+        SkDebugf("Serialized SkSL failed to compile. Ignoring/dropping SkSL blender.\n");
+        return nullptr;
+    }
+#endif
+
     return effect->makeBlender(std::move(uniforms), SkMakeSpan(children));
 }