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

Description Jer Noble 2020-07-31 10:19:57 PDT
[Mac] YouTube does not offer HDR variants to devices which support HDR
Comment 1 Jer Noble 2020-07-31 12:31:26 PDT
<rdar://problem/65188503>
Comment 2 Jer Noble 2020-07-31 12:32:15 PDT
Created attachment 405720 [details]
Patch
Comment 3 Jer Noble 2020-07-31 12:56:34 PDT
Created attachment 405725 [details]
Patch
Comment 4 Eric Carlson 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.
Comment 5 Eric Carlson 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...
Comment 6 Jer Noble 2020-07-31 14:10:35 PDT
Created attachment 405737 [details]
Patch for landing
Comment 7 Jer Noble 2020-07-31 14:52:50 PDT
Created attachment 405744 [details]
Patch for landing
Comment 8 EWS 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].
Comment 9 Ryan Haddad 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
Comment 10 Jer Noble 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.
Comment 11 Jer Noble 2020-07-31 22:58:05 PDT
Committed follow-up build fix in r265172 <https://trac.webkit.org/r265172>.