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: | P2 | 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
Ryan Haddad
2016-11-02 13:48:09 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 Marked media/modern-media-controls/pip-support/pip-support-enabled.html as flaky in http://trac.webkit.org/projects/webkit/changeset/208516 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 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 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. 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. Exiting PiP before exiting media/modern-media-controls/pip-support/pip-support-click.html avoids the crash. Created attachment 299625 [details]
Patch
Let's keep this around and apply the workaround in https://bugs.webkit.org/show_bug.cgi?id=167381. Once this is fixed, we should revert the fix for https://bugs.webkit.org/show_bug.cgi?id=167381. Created attachment 305511 [details]
Patch
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. Created attachment 308566 [details]
Patch for landing.
(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. 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 Created attachment 308567 [details]
Patch for landing.
Comment on attachment 308567 [details] Patch for landing. Clearing flags on attachment: 308567 Committed r215951: <http://trac.webkit.org/changeset/215951> (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 (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. (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. |