Bug 215022 - [Mac] YouTube does not offer HDR variants to devices which support HDR
Summary: [Mac] YouTube does not offer HDR variants to devices which support HDR
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: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-31 10:19 PDT by Jer Noble
Modified: 2020-07-31 22:58 PDT (History)
8 users (show)

See Also:


Attachments
Patch (45.85 KB, patch)
2020-07-31 12:32 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (46.10 KB, patch)
2020-07-31 12:56 PDT, Jer Noble
eric.carlson: review+
Details | Formatted Diff | Diff
Patch for landing (46.88 KB, patch)
2020-07-31 14:10 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (46.88 KB, patch)
2020-07-31 14:52 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

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