New issue
Advanced search Search tips

Issue 804206 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Compat



Sign in to add a comment

Paint API example parameter-checkerboard crashes

Reported by dirk.w.s...@gmail.com, Jan 22 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3327.0 Safari/537.36

Example URL:
https://googlechromelabs.github.io/houdini-samples/paint-worklet/parameter-checkerboard/

Steps to reproduce the problem:
1. Open the website https://googlechromelabs.github.io/houdini-samples/paint-worklet/parameter-checkerboard/

What is the expected behavior?
Website opens. Website should contain a text box with a checkerboard background drawn by the CSS Paint API.

What went wrong?
I see the text box for a brief moment w/o background then the site crashes with a "Aw, snap!". Checked on 3 different versions of Chrome Canary.

Does it occur on multiple sites: N/A

Is it a problem with a plugin? No 

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 66.0.3327.0  Channel: canary
OS Version: OS X 10.13.2
Flash Version: 

Here the log:

[51555:775:0122/063221.642657:INFO:cpu_info.cc(50)] Available number of cores: 8
[51127:84483:0122/063221.769217:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: https://googlechromelabs.github.io/houdini-samples/paint-worklet/parameter-checkerboard/
[51127:84483:0122/063221.769275:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: https://googlechromelabs.github.io/houdini-samples/paint-worklet/parameter-checkerboard/
[51127:84483:0122/063221.769300:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: https://googlechromelabs.github.io/houdini-samples/paint-worklet/parameter-checkerboard/
[51136:775:0122/063221.807664:VERBOSE1:gles2_cmd_decoder.cc(3534)] GL_EXT_packed_depth_stencil supported.
[51136:775:0122/063221.810059:VERBOSE1:gles2_cmd_decoder.cc(3534)] GL_EXT_packed_depth_stencil supported.
[51127:84483:0122/063221.818678:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: blob:https://googlechromelabs.github.io/5150b167-a5a7-48c4-9ef5-3239acab7644
[51127:84483:0122/063221.818724:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: blob:https://googlechromelabs.github.io/5150b167-a5a7-48c4-9ef5-3239acab7644
[51127:84483:0122/063221.818750:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: blob:https://googlechromelabs.github.io/5150b167-a5a7-48c4-9ef5-3239acab7644
[51555:775:0122/063221.825881:VERBOSE1:V8ContextSnapshot.cpp(140)] A context is created from snapshot for main world
[51555:775:0122/063221.826089:VERBOSE1:script_context.cc(91)] Created context:
  extension id:           (none)
  frame:                  0x4616dc1d18
  URL:                    
  context_type:           WEB_PAGE
  effective extension id: (none)
  effective context type: WEB_PAGE
[51555:775:0122/063221.827046:VERBOSE1:script_context.cc(91)] Created context:
  extension id:           (none)
  frame:                  0x0
  URL:                    
  context_type:           UNSPECIFIED
  effective extension id: (none)
  effective context type: UNSPECIFIED
[51555:775:0122/063221.827981:VERBOSE1:dispatcher.cc(356)] Num tracked contexts: 1
[51555:775:0122/063221.853101:VERBOSE1:language_detection_util.cc(131)] Detected language: und
[51127:84483:0122/063221.855213:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: https://googlechromelabs.github.io/houdini-samples/paint-worklet/parameter-checkerboard/demo2.js
[51127:84483:0122/063221.855272:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: https://googlechromelabs.github.io/houdini-samples/paint-worklet/parameter-checkerboard/demo2.js
[51127:84483:0122/063221.855314:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: https://googlechromelabs.github.io/houdini-samples/paint-worklet/parameter-checkerboard/demo2.js
[51127:84483:0122/063221.855915:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: https://googlechromelabs.github.io/favicon.ico
[51127:84483:0122/063221.855964:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: https://googlechromelabs.github.io/favicon.ico
[51127:84483:0122/063221.856003:VERBOSE1:network_delegate.cc(30)] NetworkDelegate::NotifyBeforeURLRequest: https://googlechromelabs.github.io/favicon.ico
 
Components: Internals>Compositing>Animation
Owner: xidac...@chromium.org
Status: Assigned (was: Unconfirmed)
Labels: -Pri-2 Pri-1
The reason of this crash is the same as  crbug.com/803026 . I will have a fix up soon.
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/206454eb0bc1e2e2886293abf2b205a32099ba50

commit 206454eb0bc1e2e2886293abf2b205a32099ba50
Author: Xida Chen <xidachen@chromium.org>
Date: Tue Jan 23 17:02:44 2018

[PaintWorklet] Do null check for paint_arguments in CSSPaintDefinition::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 <smcgruer@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531262}
[modify] https://crrev.com/206454eb0bc1e2e2886293abf2b205a32099ba50/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp
[modify] https://crrev.com/206454eb0bc1e2e2886293abf2b205a32099ba50/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.h
[modify] https://crrev.com/206454eb0bc1e2e2886293abf2b205a32099ba50/third_party/WebKit/Source/modules/csspaint/PaintWorkletTest.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/329d49213d27cd7eaf30f52a7fbab6118a7c5c37

commit 329d49213d27cd7eaf30f52a7fbab6118a7c5c37
Author: Xida Chen <xidachen@chromium.org>
Date: Wed Jan 24 01:03:49 2018

[PaintWorklet] Build right paint callback according to paint_arguments

In my previous CL:
https://chromium-review.googlesource.com/c/chromium/src/+/879110

I simply did null check for the |paint_arguments|, and return a nullptr
when it is null. There is a better way to handle it, which is to build
the paint callback function without the |paint_arguments| if it is null.

This CL should not change any behavior. We can use the existing tests
to verify this. We already have a PaintWorkletTest for that and a bunch
of layout tests to ensure the correct behavior.

Bug:  803026 ,  804206 
Change-Id: I07b2f58dfe88ccbb5ac27d7268eb228ea101f5fc
Reviewed-on: https://chromium-review.googlesource.com/880886
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531396}
[modify] https://crrev.com/329d49213d27cd7eaf30f52a7fbab6118a7c5c37/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp
[modify] https://crrev.com/329d49213d27cd7eaf30f52a7fbab6118a7c5c37/third_party/WebKit/Source/modules/csspaint/PaintWorkletTest.cpp

Status: Fixed (was: Assigned)
I checked on canary, there is no crash anymore. Closing this bug.
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 25 2018

Labels: merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b88ee05cf6f15afaa0d31ae99974226cfc89295c

commit b88ee05cf6f15afaa0d31ae99974226cfc89295c
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Jan 25 01:26:34 2018

[PaintWorklet] Do null check for paint_arguments in CSSPaintDefinition::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 <smcgruer@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#531262}(cherry picked from commit 206454eb0bc1e2e2886293abf2b205a32099ba50)
Reviewed-on: https://chromium-review.googlesource.com/884553
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#82}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/b88ee05cf6f15afaa0d31ae99974226cfc89295c/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp
[modify] https://crrev.com/b88ee05cf6f15afaa0d31ae99974226cfc89295c/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.h
[modify] https://crrev.com/b88ee05cf6f15afaa0d31ae99974226cfc89295c/third_party/WebKit/Source/modules/csspaint/PaintWorkletTest.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b701c2a442d7d136287f6bb675b315a1f0ae7f12

commit b701c2a442d7d136287f6bb675b315a1f0ae7f12
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Jan 25 01:28:10 2018

[PaintWorklet] Build right paint callback according to paint_arguments

In my previous CL:
https://chromium-review.googlesource.com/c/chromium/src/+/879110

I simply did null check for the |paint_arguments|, and return a nullptr
when it is null. There is a better way to handle it, which is to build
the paint callback function without the |paint_arguments| if it is null.

This CL should not change any behavior. We can use the existing tests
to verify this. We already have a PaintWorkletTest for that and a bunch
of layout tests to ensure the correct behavior.

Bug:  803026 ,  804206 
Change-Id: I07b2f58dfe88ccbb5ac27d7268eb228ea101f5fc
Reviewed-on: https://chromium-review.googlesource.com/880886
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#531396}(cherry picked from commit 329d49213d27cd7eaf30f52a7fbab6118a7c5c37)
Reviewed-on: https://chromium-review.googlesource.com/884554
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#83}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/b701c2a442d7d136287f6bb675b315a1f0ae7f12/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp
[modify] https://crrev.com/b701c2a442d7d136287f6bb675b315a1f0ae7f12/third_party/WebKit/Source/modules/csspaint/PaintWorkletTest.cpp

Sign in to add a comment