Bug 215022

Summary: [Mac] YouTube does not offer HDR variants to devices which support HDR
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, eric.carlson, ews-watchlist, glenn, peng.liu6, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
eric.carlson: review+
Patch for landing
none
Patch for landing none

Jer Noble
Reported 2020-07-31 10:19:57 PDT
[Mac] YouTube does not offer HDR variants to devices which support HDR
Attachments
Patch (45.85 KB, patch)
2020-07-31 12:32 PDT, Jer Noble 		no flags
DetailsFormatted DiffDiff
Patch (46.10 KB, patch)
2020-07-31 12:56 PDT, Jer Noble 		eric.carlson: review+
DetailsFormatted DiffDiff
Patch for landing (46.88 KB, patch)
2020-07-31 14:10 PDT, Jer Noble 		no flags
DetailsFormatted DiffDiff
Patch for landing (46.88 KB, patch)
2020-07-31 14:52 PDT, Jer Noble 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Jer Noble
Comment 1 2020-07-31 12:31:26 PDT
<rdar://problem/65188503>
Jer Noble
Comment 2 2020-07-31 12:32:15 PDT
Created attachment 405720 [details] Patch
Jer Noble
Comment 3 2020-07-31 12:56:34 PDT
Created attachment 405725 [details] Patch
Eric Carlson
Comment 4 2020-07-31 13:35:43 PDT
Comment on attachment 405725 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405725&action=review > Source/WebCore/Modules/mediasource/MediaSource.cpp:724 > + if (scriptExecutionContext() && scriptExecutionContext()->isDocument() && downcast<Document>(scriptExecutionContext())->quirks().needsVP9FullRangeFlagQuirk()) Any you use `context`? > Source/WebCore/Modules/mediasource/MediaSource.cpp:1079 > + if (scriptExecutionContext() && scriptExecutionContext()->isDocument() && downcast<Document>(scriptExecutionContext())->quirks().needsVP9FullRangeFlagQuirk()) A local context variable would be better than calling `scriptExecutionContext()` multiple times.
Eric Carlson
Comment 5 2020-07-31 13:37:06 PDT
Comment on attachment 405725 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405725&action=review >> Source/WebCore/Modules/mediasource/MediaSource.cpp:724 >> + if (scriptExecutionContext() && scriptExecutionContext()->isDocument() && downcast<Document>(scriptExecutionContext())->quirks().needsVP9FullRangeFlagQuirk()) > > Any you use `context`? *Can* you use...
Jer Noble
Comment 6 2020-07-31 14:10:35 PDT
Created attachment 405737 [details] Patch for landing
Jer Noble
Comment 7 2020-07-31 14:52:50 PDT
Created attachment 405744 [details] Patch for landing
EWS
Comment 8 2020-07-31 16:40:11 PDT
Committed r265167: <https://trac.webkit.org/changeset/265167> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405744 [details].
Ryan Haddad
Comment 9 2020-07-31 21:45:38 PDT
This change broke the Windows build: C:\cygwin\worker\win10-release\build\Source\WebCore\page/Screen.cpp(90,30): error C2027: use of undefined type 'WebCore::DOMWindow' [C:\cygwin\worker\win10-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] C:\cygwin\worker\win10-release\build\Source\WebCore\page\DOMWindowProperty.h(32): message : see declaration of 'WebCore::DOMWindow' [C:\cygwin\worker\win10-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] C:\cygwin\worker\win10-release\build\Source\WebCore\page/Screen.cpp(91,8): error C3536: 'document': cannot be used before it is initialized [C:\cygwin\worker\win10-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] https://build.webkit.org/builders/Apple%20Win%2010%20Release%20%28Build%29/builds/15541/steps/compile-webkit/logs/stdio
Jer Noble
Comment 10 2020-07-31 22:52:28 PDT
(In reply to Ryan Haddad from comment #9) > This change broke the Windows build: > C:\cygwin\worker\win10-release\build\Source\WebCore\page/Screen.cpp(90,30): > error C2027: use of undefined type 'WebCore::DOMWindow' > [C:\cygwin\worker\win10- > release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] > C:\cygwin\worker\win10-release\build\Source\WebCore\page\DOMWindowProperty. > h(32): message : see declaration of 'WebCore::DOMWindow' > [C:\cygwin\worker\win10- > release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] > C:\cygwin\worker\win10-release\build\Source\WebCore\page/Screen.cpp(91,8): > error C3536: 'document': cannot be used before it is initialized > [C:\cygwin\worker\win10- > release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] > https://build.webkit.org/builders/Apple%20Win%2010%20Release%20%28Build%29/ > builds/15541/steps/compile-webkit/logs/stdio Wow, found the one file in WebCore that doesn't include Document.h.
Jer Noble
Comment 11 2020-07-31 22:58:05 PDT
Committed follow-up build fix in r265172 <https://trac.webkit.org/r265172>.
Note You need to log in before you can comment on or make changes to this bug.