Bug 232848

Summary: Factor platform specific code out of HTMLModelElement
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, cdumez, changseok, dino, esprehn+autocc, ews-watchlist, fred.wang, glenn, gyuyoung.kim, japhet, kondapallykalyan, pdr, ryuan.choi, sergio, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 233192    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Sam Weinig 2021-11-08 14:02:35 PST 
Factor platform specific code out of HTMLModelElement
Comment 1 Sam Weinig 2021-11-08 14:05:47 PST Comment hidden (obsolete)
Comment 2 Sam Weinig 2021-11-08 14:33:11 PST Comment hidden (obsolete)
Comment 3 Sam Weinig 2021-11-08 15:18:38 PST Comment hidden (obsolete)
Comment 4 Sam Weinig 2021-11-08 15:38:12 PST Comment hidden (obsolete)
Comment 5 Sam Weinig 2021-11-08 16:31:09 PST Comment hidden (obsolete)
Comment 6 Sam Weinig 2021-11-08 16:57:57 PST Comment hidden (obsolete)
Comment 7 Sam Weinig 2021-11-08 19:36:52 PST Comment hidden (obsolete)
Comment 8 Sam Weinig 2021-11-09 08:56:33 PST Comment hidden (obsolete)
Comment 9 Sam Weinig 2021-11-09 16:25:02 PST Comment hidden (obsolete)
Comment 10 Sam Weinig 2021-11-10 12:33:15 PST 
Created attachment 443849 [details]
Patch
Comment 11 Dean Jackson 2021-11-10 16:34:29 PST 
Comment on attachment 443849 [details]
Patch

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

> Source/WebCore/Modules/model-element/HTMLModelElement.cpp:225
> +    // FIXME: For the early returns here, we should probably inform the page that that things have

Typo: that that

> Source/WebKit/ChangeLog:9
> +        Move implementation details of the macOS AVKit <model> implementation
> +        into an AVKit model player implementation.

ARKit, not AVKit.

> Source/WebKit/WebProcess/Model/mac/ARKitInlinePreviewModelPlayer.mm:98
> +    auto fileName = FileSystem::encodeForFileName(createCanonicalUUIDString()) + ".usdz";

Maybe use preferredExtensionForMIMEType? but i guess either way you're hardcoding USD.
Comment 12 Sam Weinig 2021-11-11 08:50:34 PST 
Created attachment 443950 [details]
Patch
Comment 13 Sam Weinig 2021-11-11 08:57:41 PST 
(In reply to Dean Jackson from comment #11)
> Comment on attachment 443849 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=443849&action=review
> 
> > Source/WebCore/Modules/model-element/HTMLModelElement.cpp:225
> > +    // FIXME: For the early returns here, we should probably inform the page that that things have
> 
> Typo: that that

Fixed.

> 
> > Source/WebKit/ChangeLog:9
> > +        Move implementation details of the macOS AVKit <model> implementation
> > +        into an AVKit model player implementation.
> 
> ARKit, not AVKit.

Eek. Fixed. 

> 
> > Source/WebKit/WebProcess/Model/mac/ARKitInlinePreviewModelPlayer.mm:98
> > +    auto fileName = FileSystem::encodeForFileName(createCanonicalUUIDString()) + ".usdz";
> 
> Maybe use preferredExtensionForMIMEType? but i guess either way you're
> hardcoding USD.

I didn't change this one, because it was just moved code and this all needs to be removed anyway.
Comment 14 EWS 2021-11-11 10:19:48 PST 
Committed r285637 (244138@main): <https://commits.webkit.org/244138@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 443950 [details].
Comment 15 Radar WebKit Bug Importer 2021-11-11 10:20:23 PST 
<rdar://problem/85304945>