Add an IPC to return the platform supported key-system names in Android.

Add an IPC to return the platform supported key-system names in Android.

- Split widevine_key_systems.(cc|h) into andorid_key_systems.* and
  widevine_key_system.*.
- Add a method MediaDrmBridge::GetPlatformKeySystemNames().

BUG=322395
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276953

[ycheo (away), 2014-06-10 11:26:57]

Hi David & Xiaohan,

In this CL, I added an IPC to get the list of supported key system names.

But, I'm still wondering how about consolidating two IPCs.
In other words, instead of two steps of querying the list of key system names
and identifying which codecs are supported in the key system, how about just
returning all key system names and the supported codecs at once.

I'm open to either way. Could you let me know you opinions?

Thanks,
Yuncheol

[ddorwin, 2014-06-10 18:48:05]

In the description, s/the all/all the/.

https://codereview.chromium.org/320383005/diff/1/chrome/renderer/media/chrome...
File chrome/renderer/media/chrome_key_systems.cc (right):

https://codereview.chromium.org/320383005/diff/1/chrome/renderer/media/chrome...
chrome/renderer/media/chrome_key_systems.cc:15: #include
"components/cdm/renderer/android_key_systems.h"
Should be ifdef'd after other includes

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
File components/cdm/browser/cdm_message_filter_android.cc (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:71:
CdmMessageFilterAndroid::CdmMessageFilterAndroid()
This is used by Chrome and the next one is only used by AW, right?

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:73:
key_systems_.push_back(kWidevineKeySystem);
If we need special handling for Chrome, why not just pass this in from Chrome
and have a single constructor? This also avoids including Widevine directly
here.

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:76:
CdmMessageFilterAndroid::CdmMessageFilterAndroid(
This is never used.

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:86:
IPC_MESSAGE_HANDLER(CdmHostMsg_GetSupportedKeySystem,
I'm not sure CdmHostMsg is correct. It is not a message to a CDM host.
(If this changes, the CL description needs to be updated.)

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:99: message.type() ==
CdmHostMsg_GetAllSupportedKeySystemNames) {
This one is cheap - it just returns a list, right?

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:104: void
CdmMessageFilterAndroid::OnGetSupportedKeySystem(
This should probably be something like OnGetKeySystemInfo, though there is an
element of "support" and query. OnQueryKeySystemInfo?

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:127: void
CdmMessageFilterAndroid::OnGetAllSupportedKeySystemNames(
If this was OnGetPlatform... then you wouldn't need line 73 or the related
include and you could avoid having to skip widevine in the caller
(android_key_systems.cc).

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:129: *key_systems =
key_systems_;
It seems this information should come from the same place as
MediaDrmBridge::IsKeySystemSupported().

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
File components/cdm/browser/cdm_message_filter_android.h (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.h:22:
CdmMessageFilterAndroid(const std::vector<std::string> key_systems);
explicit

Why do we need two constructors?

https://codereview.chromium.org/320383005/diff/1/components/cdm/common/cdm_me...
File components/cdm/common/cdm_messages_android.h (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/common/cdm_me...
components/cdm/common/cdm_messages_android.h:32: // Synchronously get an
supported EME key system information.
nit: s/an/a/
But the comment should probably be updated per my name comments on the function
name.

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
File components/cdm/renderer/android_key_systems.cc (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.cc:22: static
SupportedKeySystemResponse GetSupportedKeySystem(
ditto on naming

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.cc:31:
DCHECK(response.compositing_codecs & content::EME_CODEC_ALL)
This only checks that there is at least one supported codec but not that there
aren't additional unsupported codecs. For that, you'd need (!(codecs & ~ALL)) or
(codecs & ALL == codecs).

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.cc:40: response =
GetSupportedKeySystem(kWidevineKeySystem);
merge with the line above.

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.cc:68: if
(response.compositing_codecs != content::EME_CODEC_NONE) {
Don't you want to support non-compositing?

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
File components/cdm/renderer/android_key_systems.h (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.h:16: void
AddOtherAndroidSupportedKeySystems(
The name is odd. Maybe AddAndroidPlatformKeySystems.

[ycheo (away), 2014-06-11 00:08:22]

PTAL.

https://codereview.chromium.org/320383005/diff/1/chrome/renderer/media/chrome...
File chrome/renderer/media/chrome_key_systems.cc (right):

https://codereview.chromium.org/320383005/diff/1/chrome/renderer/media/chrome...
chrome/renderer/media/chrome_key_systems.cc:15: #include
"components/cdm/renderer/android_key_systems.h"
On 2014/06/10 18:48:03, ddorwin wrote:
> Should be ifdef'd after other includes

Done.

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
File components/cdm/browser/cdm_message_filter_android.cc (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:71:
CdmMessageFilterAndroid::CdmMessageFilterAndroid()
On 2014/06/10 18:48:03, ddorwin wrote:
> This is used by Chrome and the next one is only used by AW, right?

That's my original intention.

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:73:
key_systems_.push_back(kWidevineKeySystem);
On 2014/06/10 18:48:03, ddorwin wrote:
> If we need special handling for Chrome, why not just pass this in from Chrome
> and have a single constructor? This also avoids including Widevine directly
> here.

Removed.

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:76:
CdmMessageFilterAndroid::CdmMessageFilterAndroid(
On 2014/06/10 18:48:04, ddorwin wrote:
> This is never used.

Removed.

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:86:
IPC_MESSAGE_HANDLER(CdmHostMsg_GetSupportedKeySystem,
On 2014/06/10 18:48:03, ddorwin wrote:
> I'm not sure CdmHostMsg is correct. It is not a message to a CDM host.
> (If this changes, the CL description needs to be updated.)

Reverted the prefix to ChromeViewHostMsg.

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:99: message.type() ==
CdmHostMsg_GetAllSupportedKeySystemNames) {
On 2014/06/10 18:48:04, ddorwin wrote:
> This one is cheap - it just returns a list, right?

Correct. Removed this from the condition.

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:104: void
CdmMessageFilterAndroid::OnGetSupportedKeySystem(
On 2014/06/10 18:48:04, ddorwin wrote:
> This should probably be something like OnGetKeySystemInfo, though there is an
> element of "support" and query. OnQueryKeySystemInfo?

Thanks for the suggestion.

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:127: void
CdmMessageFilterAndroid::OnGetAllSupportedKeySystemNames(
On 2014/06/10 18:48:04, ddorwin wrote:
> If this was OnGetPlatform... then you wouldn't need line 73 or the related
> include and you could avoid having to skip widevine in the caller
> (android_key_systems.cc).

Done.

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.cc:129: *key_systems =
key_systems_;
On 2014/06/10 18:48:04, ddorwin wrote:
> It seems this information should come from the same place as
> MediaDrmBridge::IsKeySystemSupported().

Added a method MediaDrmBridge::GetPlatformKeySystemNames().

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
File components/cdm/browser/cdm_message_filter_android.h (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/browser/cdm_m...
components/cdm/browser/cdm_message_filter_android.h:22:
CdmMessageFilterAndroid(const std::vector<std::string> key_systems);
On 2014/06/10 18:48:04, ddorwin wrote:
> explicit
> 
> Why do we need two constructors?

I changed the code that to get key system names from MediaDrm directly.

https://codereview.chromium.org/320383005/diff/1/components/cdm/common/cdm_me...
File components/cdm/common/cdm_messages_android.h (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/common/cdm_me...
components/cdm/common/cdm_messages_android.h:32: // Synchronously get an
supported EME key system information.
On 2014/06/10 18:48:04, ddorwin wrote:
> nit: s/an/a/
> But the comment should probably be updated per my name comments on the
function
> name.

Done.

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
File components/cdm/renderer/android_key_systems.cc (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.cc:22: static
SupportedKeySystemResponse GetSupportedKeySystem(
On 2014/06/10 18:48:04, ddorwin wrote:
> ditto on naming

Done.

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.cc:31:
DCHECK(response.compositing_codecs & content::EME_CODEC_ALL)
On 2014/06/10 18:48:04, ddorwin wrote:
> This only checks that there is at least one supported codec but not that there
> aren't additional unsupported codecs. For that, you'd need (!(codecs & ~ALL))
or
> (codecs & ALL == codecs).

Good catch!.

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.cc:40: response =
GetSupportedKeySystem(kWidevineKeySystem);
On 2014/06/10 18:48:04, ddorwin wrote:
> merge with the line above.

Done.

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.cc:68: if
(response.compositing_codecs != content::EME_CODEC_NONE) {
On 2014/06/10 18:48:04, ddorwin wrote:
> Don't you want to support non-compositing?
no, but the keysyste name suffix '.noncompositing' is only for WV, right?

Do you think I should merge two results compositing_codecs with
non_compositing_codecs?

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
File components/cdm/renderer/android_key_systems.h (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.h:16: void
AddOtherAndroidSupportedKeySystems(
On 2014/06/10 18:48:04, ddorwin wrote:
> The name is odd. Maybe AddAndroidPlatformKeySystems.

Done.

[ddorwin, 2014-06-11 01:54:02]

+xhwang in case he has any comments.

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
File components/cdm/renderer/android_key_systems.cc (right):

https://codereview.chromium.org/320383005/diff/1/components/cdm/renderer/andr...
components/cdm/renderer/android_key_systems.cc:68: if
(response.compositing_codecs != content::EME_CODEC_NONE) {
On 2014/06/11 00:08:22, Yuncheol Heo wrote:
> On 2014/06/10 18:48:04, ddorwin wrote:
> > Don't you want to support non-compositing?
> no, but the keysyste name suffix '.noncompositing' is only for WV, right?
> 
> Do you think I should merge two results compositing_codecs with
> non_compositing_codecs?
> 

Right.

I don't know what the use case is or even if MediaDrmBridge knows. The
non-compositing ones are problematic for normal web pages. Actually, without
support, MediaDrmBridge might dump them all in the compositing list.

This is fine for now, but we may want to discuss.

https://codereview.chromium.org/320383005/diff/40001/chrome/renderer/media/ch...
File chrome/renderer/media/chrome_key_systems.cc (right):

https://codereview.chromium.org/320383005/diff/40001/chrome/renderer/media/ch...
chrome/renderer/media/chrome_key_systems.cc:27: #if defined(OS_ANDROID)
Because of the comment at 20, this should probably before line 20.

https://codereview.chromium.org/320383005/diff/40001/components/cdm/browser/c...
File components/cdm/browser/cdm_message_filter_android.cc (right):

https://codereview.chromium.org/320383005/diff/40001/components/cdm/browser/c...
components/cdm/browser/cdm_message_filter_android.cc:15: #include
"widevine_cdm_version.h"  // In SHARED_INTERMEDIATE_DIR.
you can remove this now.

https://codereview.chromium.org/320383005/diff/40001/components/cdm/browser/c...
File components/cdm/browser/cdm_message_filter_android.h (right):

https://codereview.chromium.org/320383005/diff/40001/components/cdm/browser/c...
components/cdm/browser/cdm_message_filter_android.h:33: void
OnQueryKeySystemInfo(const SupportedKeySystemRequest& request,
On further thought, s/Info/Support/. That includes whether it's supported as
well as support info like codecs.

https://codereview.chromium.org/320383005/diff/40001/components/cdm/common/cd...
File components/cdm/common/cdm_messages_android.h (right):

https://codereview.chromium.org/320383005/diff/40001/components/cdm/common/cd...
components/cdm/common/cdm_messages_android.h:32: // Synchronously query a
supported EME key system information.
Synchronously query key system information. If the key system is supported, the
response will be populated.
(or something like that)

https://codereview.chromium.org/320383005/diff/40001/components/cdm/common/cd...
components/cdm/common/cdm_messages_android.h:38: // Synchronously get a list of
platform supported EME key system names
nit: add a '-" before "supported" if that's really what you mean.

https://codereview.chromium.org/320383005/diff/40001/components/cdm/common/cd...
components/cdm/common/cdm_messages_android.h:39: // besides WideVine.
My point was that these are a special class that are not explicitly handled by
Chrome (and thus unlikely to be spec compliant). If that was the case, then
Widevine would obviously not be included.

https://codereview.chromium.org/320383005/diff/40001/media/base/android/media...
File media/base/android/media_drm_bridge.cc (right):

https://codereview.chromium.org/320383005/diff/40001/media/base/android/media...
media/base/android/media_drm_bridge.cc:113: if
(it->first.compare(kWidevineKeySystem) != 0)
This should be commented.

https://codereview.chromium.org/320383005/diff/40001/media/base/android/media...
media/base/android/media_drm_bridge.cc:286: return
g_key_system_uuid_manager.Get().GetPlatformKeySystemNames();
This indirection makes an extra copy. :(

https://codereview.chromium.org/320383005/diff/40001/media/base/android/media...
File media/base/android/media_drm_bridge.h (right):

https://codereview.chromium.org/320383005/diff/40001/media/base/android/media...
media/base/android/media_drm_bridge.h:52: // WideVine.
Small 'V'.

However, it might be better to explain what this list is instead of calling out
specific key system(s). See my earlier comment.

[ycheo (away), 2014-06-11 07:43:43]

https://codereview.chromium.org/320383005/diff/40001/chrome/renderer/media/ch...
File chrome/renderer/media/chrome_key_systems.cc (right):

https://codereview.chromium.org/320383005/diff/40001/chrome/renderer/media/ch...
chrome/renderer/media/chrome_key_systems.cc:27: #if defined(OS_ANDROID)
On 2014/06/11 01:54:01, ddorwin wrote:
> Because of the comment at 20, this should probably before line 20.

Done.

https://codereview.chromium.org/320383005/diff/40001/components/cdm/browser/c...
File components/cdm/browser/cdm_message_filter_android.cc (right):

https://codereview.chromium.org/320383005/diff/40001/components/cdm/browser/c...
components/cdm/browser/cdm_message_filter_android.cc:15: #include
"widevine_cdm_version.h"  // In SHARED_INTERMEDIATE_DIR.
On 2014/06/11 01:54:01, ddorwin wrote:
> you can remove this now.

Done.

https://codereview.chromium.org/320383005/diff/40001/components/cdm/browser/c...
File components/cdm/browser/cdm_message_filter_android.h (right):

https://codereview.chromium.org/320383005/diff/40001/components/cdm/browser/c...
components/cdm/browser/cdm_message_filter_android.h:33: void
OnQueryKeySystemInfo(const SupportedKeySystemRequest& request,
On 2014/06/11 01:54:01, ddorwin wrote:
> On further thought, s/Info/Support/. That includes whether it's supported as
> well as support info like codecs.

Done.

https://codereview.chromium.org/320383005/diff/40001/components/cdm/common/cd...
File components/cdm/common/cdm_messages_android.h (right):

https://codereview.chromium.org/320383005/diff/40001/components/cdm/common/cd...
components/cdm/common/cdm_messages_android.h:32: // Synchronously query a
supported EME key system information.
On 2014/06/11 01:54:01, ddorwin wrote:
> Synchronously query key system information. If the key system is supported,
the
> response will be populated.
> (or something like that)

Done.

https://codereview.chromium.org/320383005/diff/40001/components/cdm/common/cd...
components/cdm/common/cdm_messages_android.h:38: // Synchronously get a list of
platform supported EME key system names
On 2014/06/11 01:54:01, ddorwin wrote:
> nit: add a '-" before "supported" if that's really what you mean.

Done.

https://codereview.chromium.org/320383005/diff/40001/components/cdm/common/cd...
components/cdm/common/cdm_messages_android.h:39: // besides WideVine.
On 2014/06/11 01:54:01, ddorwin wrote:
> My point was that these are a special class that are not explicitly handled by
> Chrome (and thus unlikely to be spec compliant). If that was the case, then
> Widevine would obviously not be included.

Done.

https://codereview.chromium.org/320383005/diff/40001/media/base/android/media...
File media/base/android/media_drm_bridge.cc (right):

https://codereview.chromium.org/320383005/diff/40001/media/base/android/media...
media/base/android/media_drm_bridge.cc:113: if
(it->first.compare(kWidevineKeySystem) != 0)
On 2014/06/11 01:54:02, ddorwin wrote:
> This should be commented.

Done.

https://codereview.chromium.org/320383005/diff/40001/media/base/android/media...
media/base/android/media_drm_bridge.cc:286: return
g_key_system_uuid_manager.Get().GetPlatformKeySystemNames();
On 2014/06/11 01:54:01, ddorwin wrote:
> This indirection makes an extra copy. :(

As I tested, GCC does the return value optimization very well.
so this may not create an extra copy.

https://codereview.chromium.org/320383005/diff/40001/media/base/android/media...
File media/base/android/media_drm_bridge.h (right):

https://codereview.chromium.org/320383005/diff/40001/media/base/android/media...
media/base/android/media_drm_bridge.h:52: // WideVine.
On 2014/06/11 01:54:02, ddorwin wrote:
> Small 'V'.
> 
> However, it might be better to explain what this list is instead of calling
out
> specific key system(s). See my earlier comment.

Done.

[ddorwin, 2014-06-11 22:26:43]

LGTM. Thanks.

[xhwang, 2014-06-11 23:12:29]

lgtm % nits

https://codereview.chromium.org/320383005/diff/80001/chrome/renderer/media/ch...
File chrome/renderer/media/chrome_key_systems.cc (right):

https://codereview.chromium.org/320383005/diff/80001/chrome/renderer/media/ch...
chrome/renderer/media/chrome_key_systems.cc:23: 
Can you keep l.18 here?

https://codereview.chromium.org/320383005/diff/80001/components/cdm/renderer/...
File components/cdm/renderer/android_key_systems.h (right):

https://codereview.chromium.org/320383005/diff/80001/components/cdm/renderer/...
components/cdm/renderer/android_key_systems.h:17:
std::vector<content::KeySystemInfo>* concrete_key_systems);
Since these are in a new file. Add some comments about what they do.

https://codereview.chromium.org/320383005/diff/80001/media/base/android/media...
File media/base/android/media_drm_bridge.cc (right):

https://codereview.chromium.org/320383005/diff/80001/media/base/android/media...
media/base/android/media_drm_bridge.cc:114: if
(it->first.compare(kWidevineKeySystem) != 0)
Can you just do 
it->first != kWidevineKeySystem ?

[ycheo (away), 2014-06-12 02:02:45]

https://codereview.chromium.org/320383005/diff/80001/chrome/renderer/media/ch...
File chrome/renderer/media/chrome_key_systems.cc (right):

https://codereview.chromium.org/320383005/diff/80001/chrome/renderer/media/ch...
chrome/renderer/media/chrome_key_systems.cc:23: 
On 2014/06/11 23:12:28, xhwang wrote:
> Can you keep l.18 here?

Done.

https://codereview.chromium.org/320383005/diff/80001/components/cdm/renderer/...
File components/cdm/renderer/android_key_systems.h (right):

https://codereview.chromium.org/320383005/diff/80001/components/cdm/renderer/...
components/cdm/renderer/android_key_systems.h:17:
std::vector<content::KeySystemInfo>* concrete_key_systems);
On 2014/06/11 23:12:28, xhwang wrote:
> Since these are in a new file. Add some comments about what they do.

Done.

https://codereview.chromium.org/320383005/diff/80001/media/base/android/media...
File media/base/android/media_drm_bridge.cc (right):

https://codereview.chromium.org/320383005/diff/80001/media/base/android/media...
media/base/android/media_drm_bridge.cc:114: if
(it->first.compare(kWidevineKeySystem) != 0)
On 2014/06/11 23:12:28, xhwang wrote:
> Can you just do 
> it->first != kWidevineKeySystem ?

Done.

[ycheo (away), 2014-06-12 02:12:53]

PTAL.

@mkosiba for android_webview/renderer/aw_key_systems.cc.

@blundell for components/cdm.gypi.

@dcheng for components/cdm/common/cdm_messages_android.h.

[blundell, 2014-06-12 07:46:54]

components/cdm.gypi LGTM

Feel free to add per-file OWNERS of cdm.gypi to //components/OWNERS (see the
existing pattern in that file)

[mkosiba (inactive), 2014-06-12 08:47:57]

android_webview/ lgtm

[dcheng, 2014-06-12 21:02:49]

On 2014/06/12 at 08:47:57, mkosiba wrote:
> android_webview/ lgtm

IPC changes LGTM

[ycheo (away), 2014-06-12 22:40:00]

The CQ bit was checked by ycheo@chromium.org

[commit-bot: I haz the power, 2014-06-12 22:43:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ycheo@chromium.org/320383005/100001

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

Change committed as 276953