Downloads and passes the poster image to Android

In order to show the poster image while casting a video, we pass the poster image URL to Android. Like favicons, it will be downloaded and be shown in the relevant activity & view.

Relevant CLs:
Blink: https://codereview.chromium.org/131763011/
Chrome for Android: https://chrome-internal-review.googlesource.com/#/c/153405/

BUG=333260
NOTRY=true

[whywhat, 2014-02-04 15:58:01]

lgtm

https://codereview.chromium.org/135863004/diff/1/content/browser/media/androi...
File content/browser/media/android/browser_media_player_manager.cc (right):

https://codereview.chromium.org/135863004/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.cc:574: false /*
is_favicon, false so that cookies will be used. */,
nit: change to

false,  // comment here, two spaces after the comma

so you don't violate the two spaces between the code and the comments rule.

https://codereview.chromium.org/135863004/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.cc:575: 0 /*
max_bitmap_size */,
ditto

https://codereview.chromium.org/135863004/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.cc:576:
base::Bind(&BrowserMediaPlayerManager::DidDownloadPoster,base::Unretained(this)));
nit: 80 chars

https://codereview.chromium.org/135863004/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.cc:586: // To be
overriden.
What does this comment mean?

[acolwell GONE FROM CHROMIUM, 2014-02-04 20:00:21]

https://codereview.chromium.org/135863004/diff/1/content/browser/media/androi...
File content/browser/media/android/browser_media_player_manager.cc (right):

https://codereview.chromium.org/135863004/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.cc:587: }
Why isn't there any code that does anything with this image? If another
implementation is supposed to override this, shouldn't OnSetPoster() be the
empty function and the derived impl do the actual load? Is there another related
CL I can look at to understand more clearly what is going on?

https://codereview.chromium.org/135863004/diff/1/content/renderer/media/andro...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/135863004/diff/1/content/renderer/media/andro...
content/renderer/media/android/webmediaplayer_android.cc:1066: poster_url_ =
poster;
Why do you need poster_url_? It doesn't appear to be used anywhere.

[cimamoglu (inactive), 2014-02-07 14:47:26]

palmer@chromium.org: Please review changes in media_player_messages_android.h

Aaron, PTAL?

https://codereview.chromium.org/135863004/diff/1/content/browser/media/androi...
File content/browser/media/android/browser_media_player_manager.cc (right):

https://codereview.chromium.org/135863004/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.cc:587: }
On 2014/02/04 20:00:21, acolwell wrote:
> Why isn't there any code that does anything with this image? If another
> implementation is supposed to override this, shouldn't OnSetPoster() be the
> empty function and the derived impl do the actual load? Is there another
related
> CL I can look at to understand more clearly what is going on?

Done. The other CL: https://chrome-internal-review.googlesource.com/#/c/153405

https://codereview.chromium.org/135863004/diff/1/content/renderer/media/andro...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/135863004/diff/1/content/renderer/media/andro...
content/renderer/media/android/webmediaplayer_android.cc:1066: poster_url_ =
poster;
On 2014/02/04 20:00:21, acolwell wrote:
> Why do you need poster_url_? It doesn't appear to be used anywhere.

Done.

[whywhat, 2014-02-13 15:08:55]

lgtm

Could you describe a little more of what you're doing altogether in the change
description. Also reference the Blink and Clank CLs there.

[palmer, 2014-02-13 21:52:26]

IPC security LGTM, but with a question.

Also yes, definitely make the CL title and description explain better what this
CL is about.

https://codereview.chromium.org/135863004/diff/50001/content/browser/media/an...
File content/browser/media/android/browser_media_player_manager.cc (right):

https://codereview.chromium.org/135863004/diff/50001/content/browser/media/an...
content/browser/media/android/browser_media_player_manager.cc:572: // To be
overridden by subclasses.
Do any such subclasses exist now?

Does it make sense to enforce that the poster URL have the same or a similar
origin, or the same URL scheme? (I am not sure.)

[acolwell GONE FROM CHROMIUM, 2014-02-13 22:17:02]

lgtm

https://codereview.chromium.org/135863004/diff/50001/content/browser/media/an...
File content/browser/media/android/browser_media_player_manager.cc (right):

https://codereview.chromium.org/135863004/diff/50001/content/browser/media/an...
content/browser/media/android/browser_media_player_manager.cc:572: // To be
overridden by subclasses.
On 2014/02/13 21:52:27, Chromium Palmer wrote:
> Do any such subclasses exist now?
Yes. In the internal Android UI codebase.

> 
> Does it make sense to enforce that the poster URL have the same or a similar
> origin, or the same URL scheme? (I am not sure.)
No. This is an image URL so it can be cross-origin. This layer shouldn't be
enforcing anything. If any enforcement needed to happen, it should be Blink
before it reaches this code or the resource loading code downstream of this
code.

[cimamoglu (inactive), 2014-02-14 12:58:38]

Updated the description.

Thanks for all the reviews!

[cimamoglu (inactive), 2014-02-14 12:58:43]

The CQ bit was checked by cimamoglu@chromium.org

[commit-bot: I haz the power, 2014-02-14 12:59:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cimamoglu@chromium.org/135863004/50001

[commit-bot: I haz the power, 2014-02-14 15:38:45]

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

[commit-bot: I haz the power, 2014-02-14 15:38:46]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[whywhat, 2014-02-14 17:12:12]

Bot failure seems irrelevant, submitting.

[whywhat, 2014-02-14 17:12:22]

The CQ bit was checked by avayvod@chromium.org

[commit-bot: I haz the power, 2014-02-14 17:14:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cimamoglu@chromium.org/135863004/50001

[commit-bot: I haz the power, 2014-02-14 17:18:57]

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

[commit-bot: I haz the power, 2014-02-14 17:18:59]

Couldn't make sense out of svn commit message:
Sending        content/browser/media/android/browser_media_player_manager.cc
Sending        content/browser/media/android/browser_media_player_manager.h
Sending        content/common/media/media_player_messages_android.h
Sending        content/renderer/media/android/renderer_media_player_manager.cc
Sending        content/renderer/media/android/renderer_media_player_manager.h
Sending        content/renderer/media/android/webmediaplayer_android.cc
Sending        content/renderer/media/android/webmediaplayer_android.h
Transmitting file data .......
Committed revision 251361.

Warning: post-commit hook failed (exit code 2) with output:
/opt/shared_hooks/post-commit.d/10_mailer.py:137: DeprecationWarning: The popen2
module is deprecated.  Use the subprocess module.
  import popen2
Post-commit mail timed out for r251361. Your commit succeeded,
but no mail was sent.