Bug 39445 – [chromium] Change WebMediaPlayer interface so buffered() isn't const

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Victoria Kirst]

In order to keep track of an accurate buffered time, buffered() needs to change from being const to non-const.

[Victoria Kirst]

Created attachment 56628 [details]
Patch

[Victoria Kirst]

NOTE: This is a two-sided change with Chromium, so Chromium will break if this change is not updated with the Chromium change.

Chromium change:
http://codereview.chromium.org/2085012/show

[Victoria Kirst]

Created attachment 56630 [details]
Patch

[WebKit Review Bot]

Attachment 56628 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2279390

[WebKit Review Bot]

Attachment 56630 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2256420

[David Levin]

Comment on attachment 56630 [details]
Patch

Two sided commits are strongly discouraged (because they require a lot of coordination and cause breakage that hides test failures, etc.). Please try to find a way to do the change without this.

Perhaps just committing the chromium side first but implementing both overloads will let you land this change after (and then finally removing one of the overloads in chromium when the deps are rolled to pick up this patch being landed).

[Victoria Kirst]

Created attachment 57118 [details]
Patch

[Victoria Kirst]

This is a part 2 of 3 one-sided changes to chrome and WebKit. Chrome has been updated, so this commit should NOT break chromium anymore.

[David Levin]

Will r=me, etc. as soon as the cr-linux turns green.

[WebKit Review Bot]

Attachment 57118 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2506040

[David Levin]

Comment on attachment 57118 [details]
Patch

r- due to chromium build break.

I think you need to change WebKit/chromium/DEPS to point to a revision that will pick up your change to chromium.

[Victoria Kirst]

Created attachment 57691 [details]
Patch

[Victoria Kirst]

Comment on attachment 57691 [details]
Patch

Should not break on chromium anymore.

[WebKit Commit Bot]

Comment on attachment 57691 [details]
Patch

Clearing flags on attachment: 57691

Committed r60592: <http://trac.webkit.org/changeset/60592>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Tony Chang]

Sorry, I had to revert this
http://trac.webkit.org/changeset/60594

We need to coordinate updating DEPS. See https://bugs.webkit.org/show_bug.cgi?id=39948 and the threads on webkit-dev and chromium-dev.

[Victoria Kirst]

Created attachment 57819 [details]
Patch

[Kent Tamura]

Comment on attachment 57819 [details]
Patch

cq- because this is two-sided change.

[Kent Tamura]

Comment on attachment 57819 [details]
Patch

Ah, I misundertood.  The Chromium-side patch was already committed.

[WebKit Commit Bot]

Comment on attachment 57819 [details]
Patch

Clearing flags on attachment: 57819

Committed r60671: <http://trac.webkit.org/changeset/60671>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Kent Tamura]

This change broken a lot of media tests.  Rolling it out.


http://test-results.appspot.com/dashboards/flakiness_dashboard.html#referringBuilder=Webkit (webkit.org)&tests=media/controls-after-reload.html,media/controls-strict.html,media/video-controls-rendering.html,media/video-display-toggle.html,media/video-volume-slider.html,media/video-error-does-not-exist.html

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#referringBuilder=Webkit Linux (webkit.org)&tests=http/tests/media/video-cancel-load.html,http/tests/media/video-error-abort.html,media/controls-after-reload.html,media/controls-drag-timebar.html,media/controls-right-click-on-timebar.html,media/controls-strict.html,media/controls-styling.html,media/event-attributes.html,media/media-startTime.html,media/remove-from-document.html,media/video-append-source.html,media/video-autoplay.html,media/video-buffered.html,media/video-controls-rendering.html,media/video-controls-transformed.html,media/video-controls-zoomed.html,media/video-controls.html,media/video-currentTime-set.html,media/video-currentTime-set2.html,media/video-currentTime.html,media/video-display-toggle.html,media/video-dom-autoplay.html,media/video-dom-src.html,media/video-duration-known-after-eos.html,media/video-error-does-not-exist.html,media/video-load-networkState.html,media/video-load-readyState.html,media/video-loop.html,media/video-muted.html,media/video-no-audio.html,media/video-no-autoplay.html,media/video-pause-empty-events.html,media/video-pause-immediately.html,media/video-play-empty-events.html,media/video-played-collapse.html,media/video-played-ranges-1.html,media/video-played-reset.html,media/video-preload.html,media/video-reverse-play-duration.html,media/video-seek-past-end-paused.html,media/video-seek-past-end-playing.html,media/video-seekable.html,media/video-seeking.html,media/video-size.html,media/video-source-error.html,media/video-src-remove.html,media/video-timeupdate-during-playback.html,media/video-timeupdate-reverse-play.html,media/video-volume-slider.html,media/video-volume.html,media/video-zoom-controls.html

[Kent Tamura]

Rolled out by r60674.

[Victoria Kirst]

Created attachment 58833 [details]
Patch

[David Levin]

Comment on attachment 58833 [details]
Patch

Please don't remove entires from the ChangeLog (even if the change was rolled out).

(If you can't get a committer to commit this w/o the ChangeLog deletion for you easily, then upload a corrected version and I'll r+,cq+ that.)

[Victoria Kirst]

Created attachment 58919 [details]
Patch

[Victoria Kirst]

Comment on attachment 58919 [details]
Patch

Fixed the changelog!

[David Levin]

Comment on attachment 58919 [details]
Patch

> +        Reviewed by David Levin.
In general don't fill this in when you put a patch up for review. The commit queue will fill it in for you.

[WebKit Commit Bot]

Comment on attachment 58919 [details]
Patch

Clearing flags on attachment: 58919

Committed r61483: <http://trac.webkit.org/changeset/61483>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.