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

Justin Fan
Reported 2020-07-09 12:56:27 PDT
[WebGL] REGRESSION (r262366): Google search photos do not render, black images
Attachments
Patch (2.94 KB, patch)
2020-07-09 13:02 PDT, Justin Fan 		no flags
DetailsFormatted DiffDiff
Patch (4.68 KB, patch)
2020-07-10 18:01 PDT, Justin Fan 		no flags
DetailsFormatted DiffDiff
Patch for landing (2.89 KB, patch)
2020-07-14 01:50 PDT, Justin Fan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Justin Fan
Comment 1 2020-07-09 12:57:18 PDT
<rdar://problem/64964922>
Justin Fan
Comment 2 2020-07-09 13:02:52 PDT
Created attachment 403906 [details] Patch
Dean Jackson
Comment 3 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.
Simon Fraser (smfr)
Comment 4 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
Justin Fan
Comment 5 2020-07-10 18:01:17 PDT
Created attachment 404030 [details] Patch
Dean Jackson
Comment 6 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::?
Justin Fan
Comment 7 2020-07-14 01:50:23 PDT
Created attachment 404215 [details] Patch for landing
Justin Fan
Comment 8 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.
EWS
Comment 9 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].
Note You need to log in before you can comment on or make changes to this bug.