Media Remoting: Remove old interstitial implementation.

Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. The new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672, 713253
Review-Url: https://codereview.chromium.org/2801853002
Cr-Commit-Position: refs/heads/master@{#465303}
Committed: https://chromium.googlesource.com/chromium/src/+/2504c4da27a272a4906a7fd1bde697c8bdf7def1

[xjz, 2017-04-05 20:49:45]

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

[commit-bot: I haz the power, 2017-04-05 20:50:24]

Dry run: 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, 2017-04-05 22:22:01]

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

[commit-bot: I haz the power, 2017-04-05 22:22:01]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[xjz, 2017-04-06 17:21:19]

Description was changed from

==========
Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame after the new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672
==========

to

==========
Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. Will commit after the new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672
==========

[xjz, 2017-04-06 17:21:19]

xjz@chromium.org changed reviewers:
+ miu@chromium.org

[xjz, 2017-04-06 17:21:55]

PTAL

[miu, 2017-04-13 02:15:11]

Comments on PS1:

https://codereview.chromium.org/2801853002/diff/1/media/remoting/BUILD.gn
File media/remoting/BUILD.gn (right):

https://codereview.chromium.org/2801853002/diff/1/media/remoting/BUILD.gn#new...
media/remoting/BUILD.gn:70: "//skia",
Can we remove the //skia dep now?

https://codereview.chromium.org/2801853002/diff/1/media/remoting/courier_rend...
File media/remoting/courier_renderer.cc (right):

https://codereview.chromium.org/2801853002/diff/1/media/remoting/courier_rend...
media/remoting/courier_renderer.cc:100:
video_renderer_sink_->PaintSingleFrame(frame);
nit: One statement please (avoids an extra ref/deref):

 
video_renderer_sink_->PaintSingleFrame(VideoFrame::CreateBlackFrame(gfx::Size(1280,
720)));

https://codereview.chromium.org/2801853002/diff/1/media/remoting/renderer_con...
File media/remoting/renderer_controller.cc (right):

https://codereview.chromium.org/2801853002/diff/1/media/remoting/renderer_con...
media/remoting/renderer_controller.cc:262: // states, an interstitial will be
shown to notify the user that local
I'm wondering if we need this last sentence anymore. Maybe we could say,
"Playback connot be resumed without reloading the page, so leave the
CourierRenderer in-place to avoid having the default renderer attempt and fail
to play the content."

WDYT?

https://codereview.chromium.org/2801853002/diff/1/media/remoting/renderer_con...
File media/remoting/renderer_controller.h (right):

https://codereview.chromium.org/2801853002/diff/1/media/remoting/renderer_con...
media/remoting/renderer_controller.h:14: #include
"third_party/skia/include/core/SkBitmap.h"
Can we remove the SkBitmap #include?

https://codereview.chromium.org/2801853002/diff/1/media/remoting/renderer_con...
media/remoting/renderer_controller.h:45: void OnSetPoster(const GURL& poster)
override {}
We should remove OnSetPoster() from the interface, and the calls to it from
WMPI. Unless something else is using it?

[xjz, 2017-04-13 23:17:39]

Description was changed from

==========
Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. Will commit after the new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672
==========

to

==========
Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. Here is the new implemenation:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672
==========

[xjz, 2017-04-13 23:18:34]

xjz@chromium.org changed reviewers:
+ sandersd@chromium.org

[xjz, 2017-04-13 23:46:11]

Description was changed from

==========
Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. Here is the new implemenation:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672
==========

to

==========
Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. The new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672
==========

[xjz, 2017-04-13 23:46:11]

xjz@chromium.org changed reviewers:
+ jochen@chromium.org

[xjz, 2017-04-13 23:48:36]

Addressed miu's comments. PTAL. Also removed the strings.

sandersd@: PTAL media/base/* and media/blink/*.

jochen@: need your RS on content/renderer/* and chrome/common/media/*. PTAL.

Thanks!

https://codereview.chromium.org/2801853002/diff/1/media/remoting/BUILD.gn
File media/remoting/BUILD.gn (right):

https://codereview.chromium.org/2801853002/diff/1/media/remoting/BUILD.gn#new...
media/remoting/BUILD.gn:70: "//skia",
On 2017/04/13 02:15:10, miu wrote:
> Can we remove the //skia dep now?

Yes. Done.

https://codereview.chromium.org/2801853002/diff/1/media/remoting/courier_rend...
File media/remoting/courier_renderer.cc (right):

https://codereview.chromium.org/2801853002/diff/1/media/remoting/courier_rend...
media/remoting/courier_renderer.cc:100:
video_renderer_sink_->PaintSingleFrame(frame);
On 2017/04/13 02:15:10, miu wrote:
> nit: One statement please (avoids an extra ref/deref):
> 
>  
>
video_renderer_sink_->PaintSingleFrame(VideoFrame::CreateBlackFrame(gfx::Size(1280,
> 720)));

Done.

https://codereview.chromium.org/2801853002/diff/1/media/remoting/renderer_con...
File media/remoting/renderer_controller.cc (right):

https://codereview.chromium.org/2801853002/diff/1/media/remoting/renderer_con...
media/remoting/renderer_controller.cc:262: // states, an interstitial will be
shown to notify the user that local
On 2017/04/13 02:15:10, miu wrote:
> I'm wondering if we need this last sentence anymore. Maybe we could say,
> "Playback connot be resumed without reloading the page, so leave the
> CourierRenderer in-place to avoid having the default renderer attempt and fail
> to play the content."
> 
> WDYT?

Done.

https://codereview.chromium.org/2801853002/diff/1/media/remoting/renderer_con...
File media/remoting/renderer_controller.h (right):

https://codereview.chromium.org/2801853002/diff/1/media/remoting/renderer_con...
media/remoting/renderer_controller.h:14: #include
"third_party/skia/include/core/SkBitmap.h"
On 2017/04/13 02:15:10, miu wrote:
> Can we remove the SkBitmap #include?

Yes. Done.

https://codereview.chromium.org/2801853002/diff/1/media/remoting/renderer_con...
media/remoting/renderer_controller.h:45: void OnSetPoster(const GURL& poster)
override {}
On 2017/04/13 02:15:10, miu wrote:
> We should remove OnSetPoster() from the interface, and the calls to it from
> WMPI. Unless something else is using it?

Done.

[xjz, 2017-04-13 23:48:47]

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

[commit-bot: I haz the power, 2017-04-13 23:49:20]

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

[sandersd (OOO until July 31), 2017-04-14 00:08:14]

media/base and media/blink lgtm.

[commit-bot: I haz the power, 2017-04-14 02:01:39]

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

[commit-bot: I haz the power, 2017-04-14 02:01:40]

Dry run: This issue passed the CQ dry run.

[miu, 2017-04-14 22:05:32]

lgtm

[xjz, 2017-04-17 16:40:50]

jochen@: Friendly ping. Need your RS on content change. Thanks!

[jochen (gone - plz use gerrit), 2017-04-18 10:51:45]

lgtm

[xjz, 2017-04-18 15:59:37]

The CQ bit was checked by xjz@chromium.org

[commit-bot: I haz the power, 2017-04-18 16:00:04]

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, 2017-04-18 17:24:13]

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

[commit-bot: I haz the power, 2017-04-18 17:24:14]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[xjz, 2017-04-18 17:55:22]

The CQ bit was checked by xjz@chromium.org

[commit-bot: I haz the power, 2017-04-18 17:55:48]

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, 2017-04-18 18:50:19]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492538122046910,
"parent_rev": "e3b1b256fd85fa0c846e49240e681d4f5cee91f7", "commit_rev":
"2504c4da27a272a4906a7fd1bde697c8bdf7def1"}

[commit-bot: I haz the power, 2017-04-18 18:50:37]

Description was changed from

==========
Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. The new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672
==========

to

==========
Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. The new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672

Review-Url: https://codereview.chromium.org/2801853002
Cr-Commit-Position: refs/heads/master@{#465303}
Committed:
https://chromium.googlesource.com/chromium/src/+/2504c4da27a272a4906a7fd1bde6...
==========

[commit-bot: I haz the power, 2017-04-18 18:50:39]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/2504c4da27a272a4906a7fd1bde6...

[xjz, 2017-04-19 17:31:20]

Description was changed from

==========
Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. The new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672

Review-Url: https://codereview.chromium.org/2801853002
Cr-Commit-Position: refs/heads/master@{#465303}
Committed:
https://chromium.googlesource.com/chromium/src/+/2504c4da27a272a4906a7fd1bde6...
==========

to

==========
Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. The new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG=700572, 697672, 713253

Review-Url: https://codereview.chromium.org/2801853002
Cr-Commit-Position: refs/heads/master@{#465303}
Committed:
https://chromium.googlesource.com/chromium/src/+/2504c4da27a272a4906a7fd1bde6...
==========