[PaintWorklet] Do null check for paint_arguments in CSSPaintDefinitio…

…n::Paint

Currently we have shipped the CSSPaintAPI, but not CSSPaintAPIArguments.
As a result, we could skip parsing the arguments if we run chromium without
--enable-experimental-web-platform-features, then the |paint_arguments|
in the CSSPaintDefinition::Paint function becomes nullptr, and we will
hit a DCHECK.

To fix it, we always check whether it is nullptr or not in that function.
We added a unit test to ensure that it will never crash.

Bug: 803026, 804206
Change-Id: I7f4b46eea423768974c7ffb3cd691484b1ad683d
Reviewed-on: https://chromium-review.googlesource.com/879110
Reviewed-by: Stephen McGruer <[email protected]>
Commit-Queue: Xida Chen <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#531262}(cherry picked from commit 206454e)
Reviewed-on: https://chromium-review.googlesource.com/884553
Reviewed-by: Xida Chen <[email protected]>
Cr-Commit-Position: refs/branch-heads/3325@{#82}
Cr-Branched-From: bc084a8-refs/heads/master@{#530369}

third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp

@@ -69,7 +69,8 @@ scoped_refptr<Image> CSSPaintDefinition::Paint(
     const ImageResourceObserver& client,
     const IntSize& container_size,
     const CSSStyleValueVector* paint_arguments) {
-  DCHECK(paint_arguments);
+  if (!paint_arguments)
+    return nullptr;
 
   // TODO: Break dependency on LayoutObject. Passing the Node should work.
   const LayoutObject& layout_object = static_cast<const LayoutObject&>(client);

third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.h

@@ -8,6 +8,7 @@
 #include "core/CSSPropertyNames.h"
 #include "core/css/CSSSyntaxDescriptor.h"
 #include "core/css/cssom/CSSStyleValue.h"
+#include "modules/ModulesExport.h"
 #include "modules/csspaint/PaintRenderingContext2DSettings.h"
 #include "platform/bindings/ScriptWrappable.h"
 #include "platform/bindings/TraceWrapperMember.h"
@@ -25,7 +26,7 @@ class ImageResourceObserver;
 // Represents a javascript class registered on the PaintWorkletGlobalScope by
 // the author. It will store the properties for invalidation and input argument
 // types as well.
-class CSSPaintDefinition final
+class MODULES_EXPORT CSSPaintDefinition final
     : public GarbageCollectedFinalized<CSSPaintDefinition>,
       public TraceWrapperBase {
  public:

third_party/WebKit/Source/modules/csspaint/PaintWorkletTest.cpp

@@ -9,6 +9,7 @@
 #include "bindings/core/v8/V8GCController.h"
 #include "bindings/core/v8/WorkerOrWorkletScriptController.h"
 #include "core/frame/LocalFrame.h"
+#include "core/layout/LayoutView.h"
 #include "core/testing/PageTestBase.h"
 #include "modules/csspaint/CSSPaintDefinition.h"
 #include "modules/csspaint/PaintWorkletGlobalScope.h"
@@ -57,6 +58,10 @@ class PaintWorkletTest : public PageTestBase {
     return PaintWorkletGlobalScopeProxy::From(proxy_.Get());
   }
 
+  ImageResourceObserver* GetImageResourceObserver() {
+    return GetDocument().domWindow()->GetFrame()->ContentLayoutObject();
+  }
+
   // Helper function used in GlobalScopeSelection test.
   void ExpectSwitchGlobalScope(bool expect_switch_within_frame,
                                size_t num_paint_calls,
@@ -133,6 +138,28 @@ TEST_F(PaintWorkletTest, GarbageCollectionOfCSSPaintDefinition) {
   DCHECK(handle.IsEmpty());
 }
 
+// This is a crash test for crbug.com/803026. At some point, we shipped the
+// CSSPaintAPI without shipping the CSSPaintAPIArguments, the result of it is
+// that the |paint_arguments| in the CSSPaintDefinition::Paint() becomes
+// nullptr and we need to null check that. This is a regression test to ensure
+// that we don't crash.
+TEST_F(PaintWorkletTest, PaintWithNullPaintArguments) {
+  PaintWorkletGlobalScope* global_scope = GetProxy()->global_scope();
+  global_scope->ScriptController()->Evaluate(
+      ScriptSourceCode("registerPaint('foo', class { paint() { } });"));
+
+  CSSPaintDefinition* definition = global_scope->FindDefinition("foo");
+  ASSERT_TRUE(definition);
+
+  ImageResourceObserver* observer = GetImageResourceObserver();
+  ASSERT_TRUE(observer);
+
+  const IntSize container_size(100, 100);
+  scoped_refptr<Image> image =
+      definition->Paint(*observer, container_size, nullptr);
+  EXPECT_EQ(image, nullptr);
+}
+
 // In this test, we set a list of "paints_to_switch" numbers, and in each frame,
 // we switch to a new global scope when the number of paint calls is >= the
 // corresponding number.