Bug 164336 – ARGUMENT BAD: time, time >= 0

• Component: Media
• Status: NEW

[Ryan Haddad]

Encountering this assertion failure on Sierra Debug WK1 with LayoutTest media/modern-media-controls/pip-support/pip-support-enabled.html

https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK1%20(Tests)/r208291%20(1060)/results.html

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Fmodern-media-controls%2Fpip-support%2Fpip-support-enabled.html

ARGUMENT BAD: time, time >= 0
/Volumes/Data/slave/sierra-debug/build/Source/WebCore/platform/mac/WebVideoFullscreenHUDWindowController.mm(561) : NSString *timeToString(double)
1   0x10e2e09dd WTFCrash
2   0x116314f3a timeToString(double)
3   0x116314ec8 -[WebVideoFullscreenHUDWindowController remainingTimeText]
4   0x1163142f8 -[WebVideoFullscreenHUDWindowController updateTime]
5   0x116312e26 -[WebVideoFullscreenHUDWindowController scheduleTimeUpdate]
6   0x116313077 -[WebVideoFullscreenHUDWindowController fadeWindowIn]
7   0x116310347 -[WebVideoFullscreenController windowDidEnterFullscreen]
8   0x116311907 -[WebVideoFullscreenWindow animatedResizeDidEnd]
9   0x116312031 -[WebVideoFullscreenWindow animationDidEnd:]
10  0x7fff9acc5f2a __NSThreadPerformPerform
11  0x7fff9927a581 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
12  0x7fff9925b907 __CFRunLoopDoSources0
13  0x7fff9925ae76 __CFRunLoopRun
14  0x7fff9925a874 CFRunLoopRunSpecific
15  0x10be69439 runTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)
16  0x10be67b5d runTestingServerLoop()
17  0x10be670aa dumpRenderTree(int, char const**)
18  0x10be69dad DumpRenderTreeMain(int, char const**)
19  0x10bed0fe2 main
20  0x7fffae32c255 start
21  0x2

[Ryan Haddad]

Same assertion with media/modern-media-controls/remaining-time-support/remaining-time-support.html here:

https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK1%20(Tests)/r208302%20(1066)/results.html

[Ryan Haddad]

Marked media/modern-media-controls/pip-support/pip-support-enabled.html as flaky in http://trac.webkit.org/projects/webkit/changeset/208516

[Ryan Haddad]

media/modern-media-controls/play-pause-button/play-pause-button.html

https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK1%20(Tests)/r208574%20(1190)/results.html

[Radar WebKit Bug Importer]

<rdar://problem/29314891>

[Ryan Haddad]

Since modern-media-controls tests are enabled now, this is happening again.

Marked test as crashing on macOS WK1 debug in http://trac.webkit.org/projects/webkit/changeset/210995

[Antoine Quint]

media/modern-media-controls/pip-support/pip-support-enabled.html does not crash on WK1 in isolation. It must be some remnant state created by a previous test that causes the bad state.

[Antoine Quint]

Right, it's running both media/modern-media-controls/pip-support/pip-support-click.html and media/modern-media-controls/pip-support/pip-support-enabled.html in a row that causes the crash.

[Antoine Quint]

Exiting PiP before exiting media/modern-media-controls/pip-support/pip-support-click.html avoids the crash.

[Antoine Quint]

Created attachment 299625 [details]
Patch

[Antoine Quint]

Let's keep this around and apply the workaround in https://bugs.webkit.org/show_bug.cgi?id=167381.

[Antoine Quint]

Once this is fixed, we should revert the fix for https://bugs.webkit.org/show_bug.cgi?id=167381.

[Jeremy Jones]

Created attachment 305511 [details]
Patch

[Jon Lee]

Comment on attachment 305511 [details]
Patch

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

> LayoutTests/media/modern-media-controls/pip-support/pip-support-click.html:35
>      // test pip-support-enabled.html may crash, see webkit.org/b/164336.

We should remove the FIXME.

[Jeremy Jones]

Created attachment 308566 [details]
Patch for landing.

[Jeremy Jones]

(In reply to Jon Lee from comment #13)
> Comment on attachment 305511 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=305511&action=review
> 
> > LayoutTests/media/modern-media-controls/pip-support/pip-support-click.html:35
> >      // test pip-support-enabled.html may crash, see webkit.org/b/164336.
> 
> We should remove the FIXME.

Removed.

[WebKit Commit Bot]

Comment on attachment 308566 [details]
Patch for landing.

Rejecting attachment 308566 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 308566, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Erid Carlson found in /Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog does not appear to be a valid reviewer according to contributors.json.
/Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.webkit.org/results/3628027

[Jeremy Jones]

Created attachment 308567 [details]
Patch for landing.

[WebKit Commit Bot]

Comment on attachment 308567 [details]
Patch for landing.

Clearing flags on attachment: 308567

Committed r215951: <http://trac.webkit.org/changeset/215951>

[Ryan Haddad]

(In reply to WebKit Commit Bot from comment #18)
> Comment on attachment 308567 [details]
> Patch for landing.
> 
> Clearing flags on attachment: 308567
> 
> Committed r215951: <http://trac.webkit.org/changeset/215951>

media/modern-media-controls/placard-support/placard-support-pip.html seems to be crashing frequently on Sierra WK1 after this change:

https://build.webkit.org/results/Apple%20Sierra%20Release%20WK1%20(Tests)/r216023%20(1169)/media/modern-media-controls/placard-support/placard-support-pip-crash-log.txt

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Fmodern-media-controls%2Fplacard-support%2Fplacard-support-pip.html

[Jeremy Jones]

(In reply to Ryan Haddad from comment #19)
> (In reply to WebKit Commit Bot from comment #18)
> > Comment on attachment 308567 [details]
> > Patch for landing.
> > 
> > Clearing flags on attachment: 308567
> > 
> > Committed r215951: <http://trac.webkit.org/changeset/215951>
> 
> media/modern-media-controls/placard-support/placard-support-pip.html seems
> to be crashing frequently on Sierra WK1 after this change:
> 
> https://build.webkit.org/results/Apple%20Sierra%20Release%20WK1%20(Tests)/
> r216023%20(1169)/media/modern-media-controls/placard-support/placard-support-
> pip-crash-log.txt
> 
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=media%2Fmodern-media-controls%2Fplacard-
> support%2Fplacard-support-pip.html

I don't see how this change could cause a crash in fig player. The only change of substance is that this patches introduces accessing currentTime() and duration() 2x instead of once.

[Jeremy Jones]

(In reply to Jeremy Jones from comment #20)
> (In reply to Ryan Haddad from comment #19)
> > (In reply to WebKit Commit Bot from comment #18)
> > > Comment on attachment 308567 [details]
> > > Patch for landing.
> > > 
> > > Clearing flags on attachment: 308567
> > > 
> > > Committed r215951: <http://trac.webkit.org/changeset/215951>
> > 
> > media/modern-media-controls/placard-support/placard-support-pip.html seems
> > to be crashing frequently on Sierra WK1 after this change:
> > 
> > https://build.webkit.org/results/Apple%20Sierra%20Release%20WK1%20(Tests)/
> > r216023%20(1169)/media/modern-media-controls/placard-support/placard-support-
> > pip-crash-log.txt
> > 
> > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> > html#showAllRuns=true&tests=media%2Fmodern-media-controls%2Fplacard-
> > support%2Fplacard-support-pip.html
> 
> I don't see how this change could cause a crash in fig player. The only
> change of substance is that this patches introduces accessing currentTime()
> and duration() 2x instead of once.

Oh, I see. It was probably the removal of the workaround that caused the problem.