Bug 214156 - [WebGL] REGRESSION (r262366): Google search photos do not render, black images
Summary: [WebGL] REGRESSION (r262366): Google search photos do not render, black images
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Justin Fan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-09 12:56 PDT by Justin Fan
Modified: 2020-07-14 02:26 PDT (History)
8 users (show)

See Also:


Attachments
Patch (2.94 KB, patch)
2020-07-09 13:02 PDT, Justin Fan
no flags Details | Formatted Diff | Diff
Patch (4.68 KB, patch)
2020-07-10 18:01 PDT, Justin Fan
no flags Details | Formatted Diff | Diff
Patch for landing (2.89 KB, patch)
2020-07-14 01:50 PDT, Justin Fan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].