Eliminate MediaPlayer & MediaPlayerClient abstractions(readystate)

Eliminate MediaPlayer & MediaPlayerClient abstractions(readystate)

BUG=350571
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175723

[Srirama, 2014-06-05 13:51:38]

Removed MediaPlayer abstractions for readystate.
PTAL.

[Srirama, 2014-06-05 13:55:04]

Added reviewers.

[acolwell GONE FROM CHROMIUM, 2014-06-06 05:51:09]

lgtm % comments

https://codereview.chromium.org/318923004/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.h (right):

https://codereview.chromium.org/318923004/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.h:327: void
setReadyState(blink::WebMediaPlayer::ReadyState);
nit: I think it makes more sense to use ReadyState declared in this class here.
That makes readyState() and setReadyState() use the same types.

https://codereview.chromium.org/318923004/diff/1/Source/modules/imagebitmap/I...
File Source/modules/imagebitmap/ImageBitmapFactories.cpp (right):

https://codereview.chromium.org/318923004/diff/1/Source/modules/imagebitmap/I...
Source/modules/imagebitmap/ImageBitmapFactories.cpp:127: if
(!video->webMediaPlayer()) {
I don't think you will need this given that the <= HaveMetadata below should
block execution from getting to the cors check.

https://codereview.chromium.org/318923004/diff/1/Source/modules/imagebitmap/I...
Source/modules/imagebitmap/ImageBitmapFactories.cpp:135: if
(video->webMediaPlayer()->readyState() <=
blink::WebMediaPlayer::ReadyStateHaveMetadata) {
This doesn't seem right. I believe this code should be using video->readyState()
instead.

[Srirama, 2014-06-06 06:41:42]

Updated as per review comments.
abarth@: Owners review for web/, platform/ and modules/ changes please

https://codereview.chromium.org/318923004/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.h (right):

https://codereview.chromium.org/318923004/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.h:327: void
setReadyState(blink::WebMediaPlayer::ReadyState);
On 2014/06/06 05:51:09, acolwell wrote:
> nit: I think it makes more sense to use ReadyState declared in this class
here.
> That makes readyState() and setReadyState() use the same types.

Done.

https://codereview.chromium.org/318923004/diff/1/Source/modules/imagebitmap/I...
File Source/modules/imagebitmap/ImageBitmapFactories.cpp (right):

https://codereview.chromium.org/318923004/diff/1/Source/modules/imagebitmap/I...
Source/modules/imagebitmap/ImageBitmapFactories.cpp:127: if
(!video->webMediaPlayer()) {
On 2014/06/06 05:51:09, acolwell wrote:
> I don't think you will need this given that the <= HaveMetadata below should
> block execution from getting to the cors check.

Done.

https://codereview.chromium.org/318923004/diff/1/Source/modules/imagebitmap/I...
Source/modules/imagebitmap/ImageBitmapFactories.cpp:135: if
(video->webMediaPlayer()->readyState() <=
blink::WebMediaPlayer::ReadyStateHaveMetadata) {
On 2014/06/06 05:51:09, acolwell wrote:
> This doesn't seem right. I believe this code should be using
video->readyState()
> instead.

Done.

[abarth-chromium, 2014-06-06 17:14:19]

LGTM!

[Srirama, 2014-06-06 17:34:57]

The CQ bit was checked by srirama.m@samsung.com

[commit-bot: I haz the power, 2014-06-06 17:36:15]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/srirama.m@samsung.com/318923004/20001

[commit-bot: I haz the power, 2014-06-06 18:37:36]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_db...)
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/7185)
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)

[commit-bot: I haz the power, 2014-06-06 18:53:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-06 18:53:24]

Failed to apply patch for Source/platform/graphics/media/MediaPlayer.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/platform/graphics/media/MediaPlayer.h
  Hunk #1 FAILED at 118.
  1 out of 1 hunk FAILED -- saving rejects to file
Source/platform/graphics/media/MediaPlayer.h.rej

Patch:       Source/platform/graphics/media/MediaPlayer.h
Index: Source/platform/graphics/media/MediaPlayer.h
diff --git a/Source/platform/graphics/media/MediaPlayer.h
b/Source/platform/graphics/media/MediaPlayer.h
index
6de447ce9a3cbe098224143fbcf6ad98f1ee6e03..720c1ab28070306793030ec7aba841061fddea27
100644
--- a/Source/platform/graphics/media/MediaPlayer.h
+++ b/Source/platform/graphics/media/MediaPlayer.h
@@ -118,9 +118,6 @@ public:
     enum NetworkState { Empty, Idle, Loading, Loaded, FormatError,
NetworkError, DecodeError };
     virtual NetworkState networkState() const = 0;
 
-    enum ReadyState  { HaveNothing, HaveMetadata, HaveCurrentData,
HaveFutureData, HaveEnoughData };
-    virtual ReadyState readyState() const = 0;
-
     virtual double maxTimeSeekable() const = 0;
     virtual PassRefPtr<TimeRanges> buffered() const = 0;

[Srirama, 2014-06-07 01:10:06]

The CQ bit was checked by srirama.m@samsung.com

[commit-bot: I haz the power, 2014-06-07 01:10:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/srirama.m@samsung.com/318923004/20001

[commit-bot: I haz the power, 2014-06-07 03:30:14]

Change committed as 175723