Bug 232848 - Factor platform specific code out of HTMLModelElement
Summary: Factor platform specific code out of HTMLModelElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks: 233192
  Show dependency tree / graph
 
Reported: 2021-11-08 14:02 PST by Sam Weinig
Modified: 2021-11-16 08:11 PST (History)
16 users (show)

See Also:

Attachments
Patch (99.25 KB, patch)
2021-11-08 14:05 PST, Sam Weinig 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (99.28 KB, patch)
2021-11-08 14:33 PST, Sam Weinig 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (99.25 KB, patch)
2021-11-08 15:18 PST, Sam Weinig 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (99.22 KB, patch)
2021-11-08 15:38 PST, Sam Weinig 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (100.48 KB, patch)
2021-11-08 16:31 PST, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (100.33 KB, patch)
2021-11-08 16:57 PST, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (101.38 KB, patch)
2021-11-08 19:36 PST, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (101.92 KB, patch)
2021-11-09 08:56 PST, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (101.87 KB, patch)
2021-11-09 16:25 PST, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (105.00 KB, patch)
2021-11-10 12:33 PST, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (104.95 KB, patch)
2021-11-11 08:50 PST, Sam Weinig 		no flags Details | Formatted Diff | Diff
Show Obsolete (10) View All Add an attachment (proposed patch, testcase, etc.)

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