Bug 188208

Created attachment 346216 [details] Patch

Comment on attachment 346216 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346216&action=review r=me once the bots are happy > Source/WebCore/html/HTMLMediaElement.cpp:1090 > + ALWAYS_LOG(LOGIDENTIFIER, "task fired"); I think it make more sense to log from the target method, the __func__ preprocessor macro isn't very helpful inside of a lambda. > Source/WebCore/html/HTMLMediaElement.cpp:4394 > + ALWAYS_LOG(LOGIDENTIFIER, "task fired"); Ditto > Source/WebCore/html/HTMLMediaElement.cpp:5248 > + ALWAYS_LOG(LOGIDENTIFIER, "task fired"); Ditto > Source/WebCore/html/HTMLMediaElement.cpp:5557 > + m_loadMediaResourceTask.close(); > + m_configureTextTracksTask.close(); > + m_checkPlaybackTargetCompatablityTask.close(); > + m_updateMediaStateTask.close(); > + m_mediaEngineUpdatedTask.close(); > + m_updatePlayStateTask.close(); > + m_resumeTaskQueue.close(); > + m_seekTaskQueue.close(); > + m_playbackControlsManagerBehaviorRestrictionsQueue.close(); > m_seekTaskQueue.close(); > m_resumeTaskQueue.close(); > - m_shadowDOMTaskQueue.close(); > m_promiseTaskQueue.close(); > m_pauseAfterDetachedTaskQueue.close(); Nit: it looks like this is the same as the code in HTMLMediaElement::stop, might as well put it in a method you can share. > Source/WebCore/html/HTMLMediaElement.cpp:7586 > + ALWAYS_LOG(LOGIDENTIFIER, "task fired"); Ditto the comment about logging in the target method.

(In reply to Eric Carlson from comment #2) > Comment on attachment 346216 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=346216&action=review > > r=me once the bots are happy > > > Source/WebCore/html/HTMLMediaElement.cpp:1090 > > + ALWAYS_LOG(LOGIDENTIFIER, "task fired"); > > I think it make more sense to log from the target method, the __func__ > preprocessor macro isn't very helpful inside of a lambda. While that's true, doing it this way means it's possible to match up the task being scheduled to the task being fired; if we move this inside the called method, we will log whenever that method is called, regardless of whether it was scheduled or not. > > > Source/WebCore/html/HTMLMediaElement.cpp:5557 > > + m_loadMediaResourceTask.close(); > > + m_configureTextTracksTask.close(); > > + m_checkPlaybackTargetCompatablityTask.close(); > > + m_updateMediaStateTask.close(); > > + m_mediaEngineUpdatedTask.close(); > > + m_updatePlayStateTask.close(); > > + m_resumeTaskQueue.close(); > > + m_seekTaskQueue.close(); > > + m_playbackControlsManagerBehaviorRestrictionsQueue.close(); > > m_seekTaskQueue.close(); > > m_resumeTaskQueue.close(); > > - m_shadowDOMTaskQueue.close(); > > m_promiseTaskQueue.close(); > > m_pauseAfterDetachedTaskQueue.close(); > > Nit: it looks like this is the same as the code in HTMLMediaElement::stop, > might as well put it in a method you can share. Will do.

Created attachment 346634 [details] Patch

Comment on attachment 346634 [details] Patch Attachment 346634 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/8778145 New failing tests: media/video-controls-no-display-with-text-track.html

Created attachment 346639 [details] Archive of layout-test-results from ews103 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-sierra Platform: Mac OS X 10.12.6

Comment on attachment 346634 [details] Patch Attachment 346634 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/8778197 New failing tests: media/track/track-cue-rendering-horizontal.html

Created attachment 346640 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6

Comment on attachment 346634 [details] Patch Attachment 346634 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/8778279 New failing tests: media/video-controls-no-display-with-text-track.html

Created attachment 346643 [details] Archive of layout-test-results from ews117 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-sierra Platform: Mac OS X 10.12.6

Comment on attachment 346634 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346634&action=review > Source/WebCore/html/HTMLMediaElement.h:943 > + DeferrableTask<Timer> m_loadMediaResourceTask; It doesn't seem like this one is ever scheduled.

Created attachment 352470 [details] Patch

Comment on attachment 352470 [details] Patch Clearing flags on attachment: 352470 Committed r237207: <https://trac.webkit.org/changeset/237207>

All reviewed patches have been landed. Closing bug.

<rdar://problem/45322136>