Bug 211585

Summary: [macOS] Playhead in Touch Bar continues when loading stalls
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Eric Carlson 2020-05-07 11:46:42 PDT
Playhead in Touch Bar should stop when loading stalls
Comment 1 Eric Carlson 2020-05-07 11:47:11 PDT
<rdar://problem/33893306>
Comment 2 Eric Carlson 2020-05-07 11:50:52 PDT
Created attachment 398771 [details]
Patch
Comment 3 Darin Adler 2020-05-07 12:56:26 PDT
Comment on attachment 398771 [details]
Patch

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

> Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:146
> +        bool isPlaying = this->isPlaying() && !this->isStalled();

No need for the second "this->". The first one is needed because of a conflict with the name of the local variable we are setting.

> Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:436
> +    return m_mediaElement ? m_mediaElement->readyState() <= HTMLMediaElement::HAVE_CURRENT_DATA : false;

I like using && for expressions like this one.
Comment 4 Eric Carlson 2020-05-07 13:57:31 PDT
Comment on attachment 398771 [details]
Patch

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

Thanks for the review!

>> Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:146
>> +        bool isPlaying = this->isPlaying() && !this->isStalled();
> 
> No need for the second "this->". The first one is needed because of a conflict with the name of the local variable we are setting.

Fixed.

>> Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:436
>> +    return m_mediaElement ? m_mediaElement->readyState() <= HTMLMediaElement::HAVE_CURRENT_DATA : false;
> 
> I like using && for expressions like this one.

That is nicer, changed.
Comment 5 Eric Carlson 2020-05-07 14:01:25 PDT
Created attachment 398795 [details]
Patch for landing
Comment 6 EWS 2020-05-07 16:08:49 PDT
Committed r261342: <https://trac.webkit.org/changeset/261342>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398795 [details].