Bug 39445 - [chromium] Change WebMediaPlayer interface so buffered() isn't const
Summary: [chromium] Change WebMediaPlayer interface so buffered() isn't const
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 40099
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-20 13:56 PDT by Victoria Kirst
Modified: 2010-06-19 06:05 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.22 KB, patch)
2010-05-20 14:03 PDT, Victoria Kirst
no flags Details | Formatted Diff | Diff
Patch (1.41 KB, patch)
2010-05-20 14:19 PDT, Victoria Kirst
no flags Details | Formatted Diff | Diff
Patch (1.22 KB, patch)
2010-05-26 11:27 PDT, Victoria Kirst
no flags Details | Formatted Diff | Diff
Patch (1.85 KB, patch)
2010-06-02 13:29 PDT, Victoria Kirst
no flags Details | Formatted Diff | Diff
Patch (1.41 KB, patch)
2010-06-03 15:13 PDT, Victoria Kirst
no flags Details | Formatted Diff | Diff
Patch (2.17 KB, patch)
2010-06-15 16:07 PDT, Victoria Kirst
no flags Details | Formatted Diff | Diff
Patch (1.45 KB, patch)
2010-06-16 12:02 PDT, Victoria Kirst
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Victoria Kirst 2010-05-20 13:56:05 PDT
In order to keep track of an accurate buffered time, buffered() needs to change from being const to non-const.
Comment 1 Victoria Kirst 2010-05-20 14:03:23 PDT
Created attachment 56628 [details]
Patch
Comment 2 Victoria Kirst 2010-05-20 14:10:47 PDT
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
Comment 3 Victoria Kirst 2010-05-20 14:19:40 PDT
Created attachment 56630 [details]
Patch
Comment 4 WebKit Review Bot 2010-05-20 14:20:51 PDT
Attachment 56628 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2279390
Comment 5 WebKit Review Bot 2010-05-20 14:34:29 PDT
Attachment 56630 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2256420
Comment 6 David Levin 2010-05-20 15:08:22 PDT
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).
Comment 7 Victoria Kirst 2010-05-26 11:27:18 PDT
Created attachment 57118 [details]
Patch
Comment 8 Victoria Kirst 2010-05-26 11:31:58 PDT
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.
Comment 9 David Levin 2010-05-26 12:01:41 PDT
Will r=me, etc. as soon as the cr-linux turns green.
Comment 10 WebKit Review Bot 2010-05-26 12:18:21 PDT
Attachment 57118 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2506040
Comment 11 David Levin 2010-05-26 13:03:48 PDT
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.
Comment 12 Victoria Kirst 2010-06-02 13:29:00 PDT
Created attachment 57691 [details]
Patch
Comment 13 Victoria Kirst 2010-06-02 13:30:25 PDT
Comment on attachment 57691 [details]
Patch

Should not break on chromium anymore.
Comment 14 WebKit Commit Bot 2010-06-02 19:24:02 PDT
Comment on attachment 57691 [details]
Patch

Clearing flags on attachment: 57691

Committed r60592: <http://trac.webkit.org/changeset/60592>
Comment 15 WebKit Commit Bot 2010-06-02 19:24:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Tony Chang 2010-06-02 19:55:59 PDT
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.
Comment 17 Victoria Kirst 2010-06-03 15:13:58 PDT
Created attachment 57819 [details]
Patch
Comment 18 Kent Tamura 2010-06-03 21:12:02 PDT
Comment on attachment 57819 [details]
Patch

cq- because this is two-sided change.
Comment 19 Kent Tamura 2010-06-03 21:21:16 PDT
Comment on attachment 57819 [details]
Patch

Ah, I misundertood.  The Chromium-side patch was already committed.
Comment 20 WebKit Commit Bot 2010-06-04 03:24:52 PDT
Comment on attachment 57819 [details]
Patch

Clearing flags on attachment: 57819

Committed r60671: <http://trac.webkit.org/changeset/60671>
Comment 21 WebKit Commit Bot 2010-06-04 03:24:59 PDT
All reviewed patches have been landed.  Closing bug.
Comment 22 Kent Tamura 2010-06-04 04:02:44 PDT
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
Comment 23 Kent Tamura 2010-06-04 04:06:36 PDT
Rolled out by r60674.
Comment 24 Victoria Kirst 2010-06-15 16:07:52 PDT
Created attachment 58833 [details]
Patch
Comment 25 David Levin 2010-06-15 16:24:47 PDT
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.)
Comment 26 Victoria Kirst 2010-06-16 12:02:03 PDT
Created attachment 58919 [details]
Patch
Comment 27 Victoria Kirst 2010-06-16 12:03:08 PDT
Comment on attachment 58919 [details]
Patch

Fixed the changelog!
Comment 28 David Levin 2010-06-16 12:47:51 PDT
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.
Comment 29 WebKit Commit Bot 2010-06-19 06:05:45 PDT
Comment on attachment 58919 [details]
Patch

Clearing flags on attachment: 58919

Committed r61483: <http://trac.webkit.org/changeset/61483>
Comment 30 WebKit Commit Bot 2010-06-19 06:05:51 PDT
All reviewed patches have been landed.  Closing bug.