Bug 164336

Summary:

ARGUMENT BAD: time, time >= 0

Product:

WebKit

Reporter:

Ryan Haddad <ryanhaddad>

Component:

Media

Assignee:

Jeremy Jones <jeremyj-wk>

Status:

NEW

Severity:

Normal

CC:

ap, commit-queue, eric.carlson, graouts, jeremyj-wk, jer.noble, jonlee, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=167381

Bug Depends on:

Bug Blocks:

164784

Attachments:

Description	Flags
Patch	eric.carlson: review+
Patch	eric.carlson: review+, jonlee: commit-queue-
Patch for landing.	commit-queue: commit-queue-
Patch for landing.	none

Ryan Haddad

Reported 2016-11-02 13:48:09 PDT

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

Attachments
Patch (3.25 KB, patch) 2017-01-24 13:44 PST, Antoine Quint	eric.carlson: review+	Details Formatted Diff Diff
Patch (3.35 KB, patch) 2017-03-27 14:36 PDT, Jeremy Jones	eric.carlson: review+ jonlee: commit-queue-	Details Formatted Diff Diff
Patch for landing. (3.52 KB, patch) 2017-04-28 12:07 PDT, Jeremy Jones	commit-queue: commit-queue-	Details Formatted Diff Diff
Patch for landing. (3.52 KB, patch) 2017-04-28 12:10 PDT, Jeremy Jones	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Ryan Haddad

Comment 1 2016-11-02 17:01:15 PDT

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

Comment 2 2016-11-09 17:37:54 PST

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

Ryan Haddad

Comment 3 2016-11-10 20:44:42 PST

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

Comment 4 2016-11-17 11:08:28 PST

<rdar://problem/29314891>

Ryan Haddad

Comment 5 2017-01-20 16:10:06 PST

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

Comment 6 2017-01-24 13:28:02 PST

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

Comment 7 2017-01-24 13:29:29 PST

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

Comment 8 2017-01-24 13:36:28 PST

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

Antoine Quint

Comment 9 2017-01-24 13:44:12 PST

Created attachment 299625 [details] Patch

Antoine Quint

Comment 10 2017-01-24 13:49:44 PST

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

Antoine Quint

Comment 11 2017-01-24 13:52:44 PST

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

Jeremy Jones

Comment 12 2017-03-27 14:36:10 PDT

Created attachment 305511 [details] Patch

Jon Lee

Comment 13 2017-03-27 15:55:14 PDT

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

Comment 14 2017-04-28 12:07:15 PDT

Created attachment 308566 [details] Patch for landing.

Jeremy Jones

Comment 15 2017-04-28 12:07:50 PDT

(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 16 2017-04-28 12:09:08 PDT

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

Comment 17 2017-04-28 12:10:56 PDT

Created attachment 308567 [details] Patch for landing.

WebKit Commit Bot

Comment 18 2017-04-28 15:33:32 PDT

Comment on attachment 308567 [details] Patch for landing. Clearing flags on attachment: 308567 Committed r215951: <http://trac.webkit.org/changeset/215951>

Ryan Haddad

Comment 19 2017-05-01 17:15:32 PDT

(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

Comment 20 2017-05-04 10:54:44 PDT

(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

Comment 21 2017-05-04 10:57:50 PDT

(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.

Note You need to log in before you can comment on or make changes to this bug.