Media Remoting: Draw remoting interstitial on poster image.

media/remoting/remoting_renderer_controller.cc

Index: media/remoting/remoting_renderer_controller.cc
diff --git a/media/remoting/remoting_renderer_controller.cc b/media/remoting/remoting_renderer_controller.cc
index 5f0d37eacdbc7a5939d49d5b0941e7077d07da8f..503005d2880a4c726959e97c6f65ca45fefb55f8 100644
--- a/media/remoting/remoting_renderer_controller.cc
+++ b/media/remoting/remoting_renderer_controller.cc
@@ -73,6 +73,26 @@ void RemotingRendererController::OnSetCdm(CdmContext* cdm_context) {
   UpdateAndMaybeSwitch();
 }
 
+void RemotingRendererController::OnSetPoster(const GURL& poster_url) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  const GURL old_url = poster_url_;
+  poster_url_ = poster_url;
+
+  // TODO(xjz): Only download the image when we need to draw/update remoting

[miu, 2016/12/20 23:34:22]

I'm not sure this is what we would want. It could be good to download the poster
image now; and then later, once we *can* draw a remoting interstitial, the
poster image will already be downloaded and ready to use.

[xjz, 2016/12/21 01:28:40]

hmm, I think we don't want to download and keep a copy of the image for local
playback or mirroring. So the downloading will only be done if the interstitial
callback is set. This introduces latency to show the poster image as background
of interstitial when remoting starts. But the latency is only introduced once.
The interstitial will be drawn on black image first and the background is
updated later once the downloading completes. Does this sgty?

+  // interstitial. Early return if the show interstitial callback is not set.
+  // https://codereview.chromium.org/2566223005/.
+  if (download_poster_cb_.is_null())
+    return;
+
+  if (old_url != poster_url_) {
+    download_poster_cb_.Run(
+        poster_url_,
+        base::Bind(&RemotingRendererController::OnPosterImageDownloaded,
+                   weak_factory_.GetWeakPtr(), poster_url_));
+  }
+}
+
 void RemotingRendererController::SetSwitchRendererCallback(
     const base::Closure& cb) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -243,4 +263,30 @@ void RemotingRendererController::UpdateAndMaybeSwitch() {
   }
 }
 
+void RemotingRendererController::SetDownloadPosterCallback(
+    const DownloadPosterCallback& cb) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(download_poster_cb_.is_null());
+  download_poster_cb_ = cb;

[miu, 2016/12/20 23:34:22]

This just occurred to me: What if the poster URL is set *before* this method is
called? (It's technically possible, since this method isn't called until after
WMPI is created, and WMPI is allowed to call OnSetPoster() beforehand.)

I would suggest adding at the end:

  if (!poster_url_.empty())
    download_poster_cb_.Run(...);

[xjz, 2016/12/21 01:28:40]

Done. I didn't realize this. Thanks for pointing this out.

+}
+
+void RemotingRendererController::UpdateInterstitial(const SkBitmap& image) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (pipeline_metadata_.natural_size.IsEmpty() || !remote_rendering_started_)
+    return;
+
+  // TODO(xjz): Update interstitial.
+  // https://codereview.chromium.org/2566223005/
+}
+
+void RemotingRendererController::OnPosterImageDownloaded(
+    const GURL& download_url,
+    const SkBitmap& image) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (download_url != poster_url_)
+    return;  // The poster image URL has changed during the download.
+  UpdateInterstitial(image);
+}
+
 }  // namespace media