Delete browser MSE implementation.

Delete browser MSE implementation.

Spitzer can handle all MSE playbacks now. Saves about 70kb!

BUG=570711
Committed: https://crrev.com/c72600f07436a0429d107cbc75442bd585dace20
Cr-Commit-Position: refs/heads/master@{#421953}

[DaleCurtis, 2016-09-29 18:22:57]

dalecurtis@chromium.org changed reviewers:
+ nick@chromium.org, qinmin@chromium.org, wolenetz@chromium.org

[DaleCurtis, 2016-09-29 18:22:58]

Min, Matt: I figured you would like to do the honors on this one!
Nick: content/* review of everything not under a /media/ sub-folder.

[DaleCurtis, 2016-09-29 18:23:12]

The CQ bit was checked by dalecurtis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 18:23:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[DaleCurtis, 2016-09-29 18:25:15]

dalecurtis@chromium.org changed reviewers:
+ dcheng@chromium.org

[DaleCurtis, 2016-09-29 18:25:16]

+dcheng for content/common/media/media_player_messages_android.h deletions.

[dcheng, 2016-09-29 18:36:35]

LGTM with nit

https://codereview.chromium.org/2283493003/diff/20001/content/renderer/media/...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/2283493003/diff/20001/content/renderer/media/...
content/renderer/media/android/webmediaplayer_android.cc:246:
CHECK_EQ(load_type, LoadTypeURL)
Any reason this is CHECK and not a DCHECK? We generally prefer DCHECK unless
there's a good reason not to use it:
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

[commit-bot: I haz the power, 2016-09-29 18:44:22]

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

[commit-bot: I haz the power, 2016-09-29 18:44:23]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[DaleCurtis, 2016-09-29 18:45:32]

https://codereview.chromium.org/2283493003/diff/20001/content/renderer/media/...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/2283493003/diff/20001/content/renderer/media/...
content/renderer/media/android/webmediaplayer_android.cc:246:
CHECK_EQ(load_type, LoadTypeURL)
On 2016/09/29 at 18:36:35, dcheng wrote:
> Any reason this is CHECK and not a DCHECK? We generally prefer DCHECK unless
there's a good reason not to use it:
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

The reason was because I believe this path to be completely impossible, but
wanted field data in case it wasn't. This code sadly doesn't have a lot of
tests, so I'm not confident bot tests would catch all cases.

I don't feel strongly either way though, at worst this becomes a playback error
instead of a crash and we're going to delete all this code soon too. So will
change to a DCHECK once other reviewers have made their comments.

[DaleCurtis, 2016-09-29 18:51:41]

The CQ bit was checked by dalecurtis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 18:52:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[qinmin, 2016-09-29 18:56:31]

lgtm % comments: i didn't see MediaSourcePlayer get deleted in this CL

[DaleCurtis, 2016-09-29 19:24:47]

The CQ bit was checked by dalecurtis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 19:25:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[DaleCurtis, 2016-09-29 19:25:27]

Whoops, thanks for pointing that out; it was lost during the rebase.

[commit-bot: I haz the power, 2016-09-29 20:51:59]

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

[commit-bot: I haz the power, 2016-09-29 20:52:01]

Dry run: This issue passed the CQ dry run.

[ncarter (slow), 2016-09-29 21:17:08]

https://codereview.chromium.org/2283493003/diff/60001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2283493003/diff/60001/content/renderer/render...
content/renderer/render_frame_impl.cc:796: // Always use WMPI for playing blob
URLs since WMPA could never play them very
Is this hunk supposed to be part of this CL?

I don't immediately see how this is connected to the code deletion. There's no
SchemeIsBlob() in the deleted lines of the diff, so this check doesn't appear to
be relocated code.

[DaleCurtis, 2016-09-29 21:55:54]

https://codereview.chromium.org/2283493003/diff/60001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2283493003/diff/60001/content/renderer/render...
content/renderer/render_frame_impl.cc:796: // Always use WMPI for playing blob
URLs since WMPA could never play them very
On 2016/09/29 at 21:17:08, ncarter wrote:
> Is this hunk supposed to be part of this CL?
> 
> I don't immediately see how this is connected to the code deletion. There's no
SchemeIsBlob() in the deleted lines of the diff, so this check doesn't appear to
be relocated code.

Yes. All MSE playbacks are blob:// but we still have a chrome://flag for
disabling the unified media pipeline. Without this users who disable via the
flag for testing purposes will lose all MSE playbacks; this whole function is
something that will go away once we can delete the flag and
WebMediaPlayerAndroid.

[ncarter (slow), 2016-09-29 22:00:21]

lgtm

[DaleCurtis, 2016-09-29 22:14:14]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2016-09-29 22:14:14]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org,
qinmin@chromium.org
Link to the patchset: https://codereview.chromium.org/2283493003/#ps60001
(title: "Actually delete MSP. Cleanse references. Remove AudioTrack usage.")

[commit-bot: I haz the power, 2016-09-29 22:14:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 22:22:23]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-29 22:24:55]

Description was changed from

==========
Delete browser MSE implementation.

Spitzer can handle all MSE playbacks now. Saves about 70kb!

BUG=570711
==========

to

==========
Delete browser MSE implementation.

Spitzer can handle all MSE playbacks now. Saves about 70kb!

BUG=570711

Committed: https://crrev.com/c72600f07436a0429d107cbc75442bd585dace20
Cr-Commit-Position: refs/heads/master@{#421953}
==========

[commit-bot: I haz the power, 2016-09-29 22:24:56]

Patchset 4 (id:??) landed as
https://crrev.com/c72600f07436a0429d107cbc75442bd585dace20
Cr-Commit-Position: refs/heads/master@{#421953}

[wolenetz, 2016-09-30 23:23:24]

belated LGTM % some nits:

https://codereview.chromium.org/2283493003/diff/60001/content/common/media/me...
File content/common/media/media_player_messages_android.h (left):

https://codereview.chromium.org/2283493003/diff/60001/content/common/media/me...
content/common/media/media_player_messages_android.h:112: // keyframe. See
http://crbug.com/304234.
Yay for this removal. :)

https://codereview.chromium.org/2283493003/diff/60001/content/renderer/media/...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/2283493003/diff/60001/content/renderer/media/...
content/renderer/media/android/webmediaplayer_android.cc:147:
ignore_metadata_duration_change_(false),
Without MSE, this field and associated conditions are no longer necessary in
WMPA.

https://codereview.chromium.org/2283493003/diff/60001/content/renderer/media/...
content/renderer/media/android/webmediaplayer_android.cc:1240:
blink::WebContentDecryptionModuleExceptionInvalidStateError, 0,
nit: blink::WebContentDecryptionModuleExceptionNotSupportedError ? I'm not sure
-- jrummell@ / ddorwin@ / xhwang@ will know better, but completeWithError has
promise resolution condition attached only to the NotSupportedError IIUC. Also,
the default impl of this method in the base class is: 
result.completeWithError(WebContentDecryptionModuleExceptionNotSupportedError,
0, "ERROR");

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
File media/base/android/media_codec_bridge.h (right):

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
media/base/android/media_codec_bridge.h:106: // Same QueueSecureInputBuffer
overriden for the use with MediaCodecPlayer.
But.. no MediaCodecPlayer is in the code as of Aug 25 IIUC.

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
File media/base/android/media_statistics.cc (left):

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
media/base/android/media_statistics.cc:97: "Media.MSE.PlaybackDuration",
duration,
Deprecate this in histograms.xml?

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
media/base/android/media_statistics.cc:105: "Media.MSE.LateAudioFrames",
ditto

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
media/base/android/media_statistics.cc:111: "Media.MSE.LateVideoFrames",
ditto

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
media/base/android/media_statistics.cc:120:
UMA_HISTOGRAM_COUNTS("Media.MSE.Starvations",
ditto

[DaleCurtis, 2016-09-30 23:34:24]

Thanks! Followup here https://codereview.chromium.org/2382023005/

https://codereview.chromium.org/2283493003/diff/60001/content/renderer/media/...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/2283493003/diff/60001/content/renderer/media/...
content/renderer/media/android/webmediaplayer_android.cc:147:
ignore_metadata_duration_change_(false),
On 2016/09/30 23:23:24, wolenetz wrote:
> Without MSE, this field and associated conditions are no longer necessary in
> WMPA.

Removed.

https://codereview.chromium.org/2283493003/diff/60001/content/renderer/media/...
content/renderer/media/android/webmediaplayer_android.cc:1240:
blink::WebContentDecryptionModuleExceptionInvalidStateError, 0,
On 2016/09/30 at 23:23:24, wolenetz wrote:
> nit: blink::WebContentDecryptionModuleExceptionNotSupportedError ? I'm not
sure -- jrummell@ / ddorwin@ / xhwang@ will know better, but completeWithError
has promise resolution condition attached only to the NotSupportedError IIUC.
Also, the default impl of this method in the base class is: 
result.completeWithError(WebContentDecryptionModuleExceptionNotSupportedError,
0, "ERROR");

Deleted.

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
File media/base/android/media_codec_bridge.h (right):

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
media/base/android/media_codec_bridge.h:106: // Same QueueSecureInputBuffer
overriden for the use with MediaCodecPlayer.
On 2016/09/30 23:23:24, wolenetz wrote:
> But.. no MediaCodecPlayer is in the code as of Aug 25 IIUC.

Comment updated.

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
File media/base/android/media_statistics.cc (left):

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
media/base/android/media_statistics.cc:97: "Media.MSE.PlaybackDuration",
duration,
On 2016/09/30 23:23:23, wolenetz wrote:
> Deprecate this in histograms.xml?

Done.

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
media/base/android/media_statistics.cc:105: "Media.MSE.LateAudioFrames",
On 2016/09/30 23:23:23, wolenetz wrote:
> ditto

Done.

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
media/base/android/media_statistics.cc:111: "Media.MSE.LateVideoFrames",
On 2016/09/30 23:23:23, wolenetz wrote:
> ditto

Done.

https://codereview.chromium.org/2283493003/diff/60001/media/base/android/medi...
media/base/android/media_statistics.cc:120:
UMA_HISTOGRAM_COUNTS("Media.MSE.Starvations",
On 2016/09/30 23:23:23, wolenetz wrote:
> ditto

Done.