Bug 214156

Summary: [WebGL] REGRESSION (r262366): Google search photos do not render, black images
Product: WebKit Reporter: Justin Fan <justin_fan>
Component: New BugsAssignee: Justin Fan <justin_fan>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, dino, esprehn+autocc, ews-watchlist, gyuyoung.kim, simon.fraser, webkit-bug-importer
Priority: P1 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Justin Fan 2020-07-09 12:56:27 PDT 
[WebGL] REGRESSION (r262366): Google search photos do not render, black images
Comment 1 Justin Fan 2020-07-09 12:57:18 PDT 
<rdar://problem/64964922>
Comment 2 Justin Fan 2020-07-09 13:02:52 PDT 
Created attachment 403906 [details]
Patch
Comment 3 Dean Jackson 2020-07-09 13:06:41 PDT 
Comment on attachment 403906 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=403906&action=review

I think you should be able to write a pretty simple ref test for this.

<canvas>
<div></div>

draw into canvas element
canvas.remove();
divElement.appendChild(canvas);

> Source/WebCore/html/HTMLCanvasElement.cpp:1014
> +    if (insertionType.connectedToDocument)
> +        addObserver(parentOfInsertedTree.document());

Can it be our document() at this point? I expect this is called after the insertion has happened.
Comment 4 Simon Fraser (smfr) 2020-07-09 13:12:11 PDT 
Comment on attachment 403906 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=403906&action=review

> Source/WebCore/html/HTMLCanvasElement.cpp:1011
> +Node::InsertedIntoAncestorResult HTMLCanvasElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)

The cool kids write this as auto HTMLCanvasElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree) -> InsertedIntoAncestorResult
Comment 5 Justin Fan 2020-07-10 18:01:17 PDT 
Created attachment 404030 [details]
Patch
Comment 6 Dean Jackson 2020-07-10 18:15:46 PDT 
Comment on attachment 403906 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=403906&action=review

>> Source/WebCore/html/HTMLCanvasElement.cpp:1011
>> +Node::InsertedIntoAncestorResult HTMLCanvasElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
> 
> The cool kids write this as auto HTMLCanvasElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree) -> InsertedIntoAncestorResult

Why? Just to avoid Node::?
Comment 7 Justin Fan 2020-07-14 01:50:23 PDT 
Created attachment 404215 [details]
Patch for landing
Comment 8 Justin Fan 2020-07-14 01:51:15 PDT 
Wrote a ref test, but could not get it to play nice with WebKitTestRunner. Upon advice, skipping the test for now.
Comment 9 EWS 2020-07-14 02:26:07 PDT 
Committed r264334: <https://trac.webkit.org/changeset/264334>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404215 [details].