Bug 222114 - HTMLModelElement needs a renderer
Summary: HTMLModelElement needs a renderer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
: 219114 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-02-18 09:04 PST by Sam Weinig
Modified: 2021-03-03 04:46 PST (History)
19 users (show)

See Also:


Attachments
Patch (551.04 KB, patch)
2021-02-22 15:36 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (550.98 KB, patch)
2021-02-22 16:49 PST, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2021-02-18 09:04:57 PST
HTMLModelElement needs a renderer. Let's add RenderModel.
Comment 1 Sam Weinig 2021-02-22 15:36:00 PST
Created attachment 421251 [details]
Patch
Comment 2 Sam Weinig 2021-02-22 15:36:53 PST
Patch looks bigger than it is due a binary file at the bottom. Don't be scared.
Comment 3 Simon Fraser (smfr) 2021-02-22 15:42:25 PST
Comment on attachment 421251 [details]
Patch

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

> Source/WebCore/Modules/model-element/HTMLModelElement.cpp:47
> +    , m_dataComplete { false }

Remove

> Source/WebCore/Modules/model-element/HTMLModelElement.cpp:186
> +    if (resource.loadFailedOrCanceled()) {
> +        m_data = nullptr;
> +
> +        m_resource->removeClient(*this);
> +        m_resource = nullptr;
> +
> +        if (auto* renderer = this->renderer())
> +            renderer->updateFromElement();
> +
> +        m_readyPromise->reject(Exception { NetworkError });
> +    } else {
> +        m_dataComplete = true;
> +
> +        m_resource->removeClient(*this);
> +        m_resource = nullptr;
> +
> +        if (auto* renderer = this->renderer())
> +            renderer->updateFromElement();
> +
> +        m_readyPromise->resolve(*this);
> +    }

I feel like an early return here somewhere would be nicer. Share the common bits in a lambda.

> Source/WebCore/Modules/model-element/HTMLModelElement.h:75
> +    bool m_dataComplete;

{ false }
Put small things after big things to minimize padding.
Comment 4 Sam Weinig 2021-02-22 16:49:32 PST
Created attachment 421267 [details]
Patch
Comment 5 EWS 2021-02-22 17:43:32 PST
Committed r273290: <https://commits.webkit.org/r273290>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421267 [details].
Comment 6 Radar WebKit Bug Importer 2021-02-22 17:44:15 PST
<rdar://problem/74622391>
Comment 7 Aakash Jain 2021-02-23 04:45:41 PST
(In reply to EWS from comment #5)
> Committed r273290: <https://commits.webkit.org/r273290>
Following layout-tests added in this commit seems to be consistently failing on iOS:

model-element/model-element-renderer.html 
model-element/model-element-renderer-no-source.html

History: https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=model-element%2Fmodel-element-renderer-no-source.html&test=model-element%2Fmodel-element-renderer.html
Comment 8 Aakash Jain 2021-02-23 04:46:34 PST
Diff (from: https://ews-build.s3-us-west-2.amazonaws.com/iOS-14-Simulator-WK2-Tests-EWS/r421275-7501-clean-tree/model-element/model-element-renderer-no-source-diff.txt):

-layer at (0,0) size 800x170
-  RenderBlock {HTML} at (0,0) size 800x170
-    RenderBody {BODY} at (8,8) size 784x154
+layer at (0,0) size 800x171
+  RenderBlock {HTML} at (0,0) size 800x171
+    RenderBody {BODY} at (8,8) size 784x155
Comment 9 Sam Weinig 2021-02-23 13:02:01 PST
Working on the test fix in https://bugs.webkit.org/show_bug.cgi?id=222323.
Comment 10 Antoine Quint 2021-03-03 04:46:18 PST
*** Bug 219114 has been marked as a duplicate of this bug. ***