[webkit-reviews] review granted: [Bug 173030] Netflix seeking quirk should also apply to Now Playing, and should always use the livestream UI : [Attachment 312114] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Beth Dakin <bdakin at apple.com>'s
request for review:
Bug 173030: Netflix seeking quirk should also apply to Now Playing, and should
always use the livestream UI
https://bugs.webkit.org/show_bug.cgi?id=173030

Attachment 312114: Patch

https://bugs.webkit.org/attachment.cgi?id=312114&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 312114
  --> https://bugs.webkit.org/attachment.cgi?id=312114
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:7091
> +static bool needsSeekingSupportQuirk(Page& page)

It’s awkward to use this since the caller needs to check Page for null. I think
it would be cleaner to just pass an Element or Document to this function.

Also, we could write an implementation that uses Document::settings to check
the "needs quirks" flag, and then Document::topDocument to get the hose, and
then we would not need to use Page at all.

In fact, there would be no null checks needed either!

    auto& document = element.document().topDocument();
    if (!document.settings().needsSiteSpecificQuirks())
	return false;
    String host = document.url().host();
    return equalLettersIgnoringASCIICase(host, "netflix.com") ||
host.endsWithIgnoringASCIICase(".netflix.com");

> Source/WebCore/html/HTMLMediaElement.cpp:7110
> +    if (Page* page = document().page()) {
> +	   if (needsSeekingSupportQuirk(*page))
> +	       return false;
> +    }
>      return !isLiveStream();

Then this code could be:

    return !needsSeekingSupportQuirk(*this) && !isLiveStream();

> Source/WebCore/platform/mac/WebPlaybackControlsManager.mm:123
> +    return !isnan(_contentDuration) && !isinf(_contentDuration);

A nice way to write this is:

    return std::isfinite(_contentDuration);