Allow API functions and events to have entries in _api_features.json

Allow API functions and events to have entries in _api_features.json

If an API is only partially available to a context, the availability can be
specified per function or per event in _api_features.json.

BUG=55316, 120070, 232247
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=199155

[cduvall, 2013-04-26 02:07:21]

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/_api_features.json (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/_api_features.json:56: "contexts":
["blessed_extension", "unblessed_extension", "content_script"]
Not sure if this is right or not, or if it needs extension_types.

[not at google - send to devlin, 2013-04-27 01:02:42]

Despite most of my comments, the main one is in binding.js - we should look at
implementing most of the logic in there.

E.g. something like

function generate(schema) {
  var mod = null;

  function createNode(name) {
    var availability = GetAvailability(schema.name + '.' + name);
    if (!availability)
      availability = GetAvailability(schema.name);
    if (!availability.is_available)
      return null;
    mod = mod || {};
    mod[name] = {};
    return mod[name];
  }

  forEach(schema.functions, function(fn) {
    var node = createNode(fn.name);
    if (!node)
      return;
    ... set up the function on that node
  });
  forEach(schema.events, function(event) {
    ... createNode etc
  });
  forEach(schema.properties, function(name, property) {
    ... createNode etc
  });
}

very rough, hopefully you know what I mean anyway.

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/_api_features.json (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/_api_features.json:56: "contexts":
["blessed_extension", "unblessed_extension", "content_script"]
On 2013/04/26 02:07:21, cduvall wrote:
> Not sure if this is right or not, or if it needs extension_types.

It does need extension types, it's odd that this works.

In any case, not *all* of runtime can be accessed from content scripts. Only the
unprivileged ones. This should be just ["blessed_extension"] then for each
unprivileged entry also have the unblessed/content_script case.

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/extension_api.cc (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/extension_api.cc:412: Feature::Availability
ExtensionAPI::IsPartAvailableToContext(
this method should probably return a bool, since it's unclear what the or-ing
together of several availabilities is, whereas it's clear what or-ing several
bools are.

if you know what I mean...

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/extension_api.cc:424: std::string feature_name =
*i;
std::string&

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/extension_api.cc:425: if (feature_name != api_name
&& feature_name.find(api_name) == 0) {
i'm worried that at some point we'll have API names that are legitimately
subsets of others. E.g. a hypothetical appFoo API, of which app is a prefix.

I.e. use string-split or feature_name.find(api_name + ".") or something.

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/extension_api_unittest.cc (left):

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/extension_api_unittest.cc:97:
EXPECT_TRUE(extension_api->IsPrivileged("runtime.lastError"));
change to EXPECT_FALSE?

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/extensions...
File chrome/renderer/extensions/chrome_v8_context.cc (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/extensions...
chrome/renderer/extensions/chrome_v8_context.cc:158: return
ExtensionAPI::GetSharedInstance()->IsPartAvailableToContext(
seems like this might be a dangerous oversimplification

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/resources/...
File chrome/renderer/resources/extensions/binding.js (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/resources/...
chrome/renderer/resources/extensions/binding.js:281: hasEventOrFunction = true;
we may actually want to implement most of this in here, rather than the C++
stuff. It's more of a rework of the JS code but it'd be simpler overall I think.

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/resources/...
chrome/renderer/resources/extensions/binding.js:426: if
(!availability.is_available && !hasEventOrFunction) {
You should be able to calculate hasEventOrFunction by just looking at the
schema.

[cduvall, 2013-05-01 02:51:47]

As I said in a comment, I'm not sure how much more logic can be implemented in
binding.js. The C++ is necessary to determine what bindings need to be
registered, and binding.js is run after the bindings have already been
registered.

I'm also not quite sure if something like your createNode() example will work.
For example, when adding functions to mod, binding.js does this:

mod[functionDef.name] = (function() {
  var args = Array.prototype.slice.call(arguments);
  ...
}).bind(apiFunction);

Since the node that would be returned from a createNode() type function would be
what is at mod[functionDef.name], it would just be overwritten when the function
is assigned to it, and we can't just do:

node = (function() ...

because that won't actually set the node in mod.

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/_api_features.json (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/_api_features.json:56: "contexts":
["blessed_extension", "unblessed_extension", "content_script"]
On 2013/04/27 01:02:43, kalman wrote:
> On 2013/04/26 02:07:21, cduvall wrote:
> > Not sure if this is right or not, or if it needs extension_types.
> 
> It does need extension types, it's odd that this works.
> 
> In any case, not *all* of runtime can be accessed from content scripts. Only
the
> unprivileged ones. This should be just ["blessed_extension"] then for each
> unprivileged entry also have the unblessed/content_script case.

I think without extension_types, it defaults to being available in all of them.
Is there somewhere to check which contexts/extension_types each API should be
available to?

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/extension_api.cc (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/extension_api.cc:412: Feature::Availability
ExtensionAPI::IsPartAvailableToContext(
On 2013/04/27 01:02:43, kalman wrote:
> this method should probably return a bool, since it's unclear what the or-ing
> together of several availabilities is, whereas it's clear what or-ing several
> bools are.
> 
> if you know what I mean...

Done.

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/extension_api.cc:424: std::string feature_name =
*i;
On 2013/04/27 01:02:43, kalman wrote:
> std::string&

Done.

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/extension_api.cc:425: if (feature_name != api_name
&& feature_name.find(api_name) == 0) {
On 2013/04/27 01:02:43, kalman wrote:
> i'm worried that at some point we'll have API names that are legitimately
> subsets of others. E.g. a hypothetical appFoo API, of which app is a prefix.
> 
> I.e. use string-split or feature_name.find(api_name + ".") or something.

Done.

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/extension_api_unittest.cc (left):

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/extension_api_unittest.cc:97:
EXPECT_TRUE(extension_api->IsPrivileged("runtime.lastError"));
On 2013/04/27 01:02:43, kalman wrote:
> change to EXPECT_FALSE?

Done.

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/extensions...
File chrome/renderer/extensions/chrome_v8_context.cc (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/extensions...
chrome/renderer/extensions/chrome_v8_context.cc:158: return
ExtensionAPI::GetSharedInstance()->IsPartAvailableToContext(
On 2013/04/27 01:02:43, kalman wrote:
> seems like this might be a dangerous oversimplification

Maybe it would be better to rename GetPossibleAvailabilityForContext().

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/resources/...
File chrome/renderer/resources/extensions/binding.js (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/resources/...
chrome/renderer/resources/extensions/binding.js:281: hasEventOrFunction = true;
On 2013/04/27 01:02:43, kalman wrote:
> we may actually want to implement most of this in here, rather than the C++
> stuff. It's more of a rework of the JS code but it'd be simpler overall I
think.

All the C++ is just to make sure that the bindings that are needed will be
registered in the correct contexts, while only checking the features files. If
we do that in here, then API schemas will be loaded when we don't want them to
be (like in web pages).

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/resources/...
chrome/renderer/resources/extensions/binding.js:426: if
(!availability.is_available && !hasEventOrFunction) {
On 2013/04/27 01:02:43, kalman wrote:
> You should be able to calculate hasEventOrFunction by just looking at the
> schema.

Done.

[not at google - send to devlin, 2013-05-01 20:47:03]

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/_api_features.json (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/_api_features.json:56: "contexts":
["blessed_extension", "unblessed_extension", "content_script"]
On 2013/05/01 02:51:47, cduvall wrote:
> On 2013/04/27 01:02:43, kalman wrote:
> > On 2013/04/26 02:07:21, cduvall wrote:
> > > Not sure if this is right or not, or if it needs extension_types.
> > 
> > It does need extension types, it's odd that this works.
> > 
> > In any case, not *all* of runtime can be accessed from content scripts. Only
> the
> > unprivileged ones. This should be just ["blessed_extension"] then for each
> > unprivileged entry also have the unblessed/content_script case.
> 
> I think without extension_types, it defaults to being available in all of
them.
> Is there somewhere to check which contexts/extension_types each API should be
> available to?

yeah, so, these runtime APIs shouldn't be available to hosted apps, and I
believe this change will allow it to be? Either I'm wrong about that - or we
don't test it - ugh!

When constructing the features we should ensure that either it has dependencies
or it specifies extension types (transitively eventually something will check
extension types).

And I was wondering this before, but maybe features should have an "inherit"
property which imports the property from another feature. Most of these would be
much simpler then - the runtime feature would specify extension types, then all
of these sub-features would inherit from it then specify a custom context value.

WDYT? Is it worth doing and/or worth doing now or as a follow up?

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/extensions...
File chrome/renderer/extensions/chrome_v8_context.cc (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/extensions...
chrome/renderer/extensions/chrome_v8_context.cc:158: return
ExtensionAPI::GetSharedInstance()->IsPartAvailableToContext(
On 2013/05/01 02:51:47, cduvall wrote:
> On 2013/04/27 01:02:43, kalman wrote:
> > seems like this might be a dangerous oversimplification
> 
> Maybe it would be better to rename GetPossibleAvailabilityForContext().

Ah yeah good point. Something even more explicit perhaps, like
IsAnyPartAvailableToContext()? IsAnyComponent... IsAnyFunctionEventOrProperty
(kidding).. IsAnyFeatureAvailableToContext (mm sounding better)... eh,
something. But it should at least start with Is.

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/resources/...
File chrome/renderer/resources/extensions/binding.js (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/resources/...
chrome/renderer/resources/extensions/binding.js:281: hasEventOrFunction = true;
On 2013/05/01 02:51:47, cduvall wrote:
> On 2013/04/27 01:02:43, kalman wrote:
> > we may actually want to implement most of this in here, rather than the C++
> > stuff. It's more of a rework of the JS code but it'd be simpler overall I
> think.
> 
> All the C++ is just to make sure that the bindings that are needed will be
> registered in the correct contexts, while only checking the features files. If
> we do that in here, then API schemas will be loaded when we don't want them to
> be (like in web pages).

yep - sorry I was misreading this a bit. cool.

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
File chrome/renderer/resources/extensions/binding.js (right):

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:262:
addUnprivilegedAccessGetter(mod, functionDef.name);
Just leave it blank. No need for the unprivileged thing.

though I think we'll want to fix 234311 and apply it here somehow. ugh. I can't
think of any APIs that would benefit from it yet, though.

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:325: var options =
eventDef.options || {};
might as well define options after this block.

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:327:
addUnprivilegedAccessGetter(mod, eventDef.name);
ditto - no unprivileged access getter, just leave it blank.

In fact, let's just delete addUnprivilegedAccessGetter entirely.

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:359:
addUnprivilegedAccessGetter(mod, propertyName);
ditto

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:412:
(hasItemOfType(schema.events) || hasItemOfType(schema.functions))) {
a simpler test may be to just check whether the module object is empty. You can
use Object.keys(mod).length == 0.

[cduvall, 2013-05-01 23:47:10]

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/_api_features.json (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/_api_features.json:56: "contexts":
["blessed_extension", "unblessed_extension", "content_script"]
On 2013/05/01 20:47:04, kalman wrote:
> On 2013/05/01 02:51:47, cduvall wrote:
> > On 2013/04/27 01:02:43, kalman wrote:
> > > On 2013/04/26 02:07:21, cduvall wrote:
> > > > Not sure if this is right or not, or if it needs extension_types.
> > > 
> > > It does need extension types, it's odd that this works.
> > > 
> > > In any case, not *all* of runtime can be accessed from content scripts.
Only
> > the
> > > unprivileged ones. This should be just ["blessed_extension"] then for each
> > > unprivileged entry also have the unblessed/content_script case.
> > 
> > I think without extension_types, it defaults to being available in all of
> them.
> > Is there somewhere to check which contexts/extension_types each API should
be
> > available to?
> 
> yeah, so, these runtime APIs shouldn't be available to hosted apps, and I
> believe this change will allow it to be? Either I'm wrong about that - or we
> don't test it - ugh!
> 
> When constructing the features we should ensure that either it has
dependencies
> or it specifies extension types (transitively eventually something will check
> extension types).
> 
> And I was wondering this before, but maybe features should have an "inherit"
> property which imports the property from another feature. Most of these would
be
> much simpler then - the runtime feature would specify extension types, then
all
> of these sub-features would inherit from it then specify a custom context
value.
> 
> WDYT? Is it worth doing and/or worth doing now or as a follow up?

I think it would definitely be useful for pretty much any sub-feature. I'll do
it as a follow-up though.

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/extensions...
File chrome/renderer/extensions/chrome_v8_context.cc (right):

https://codereview.chromium.org/14494013/diff/2001/chrome/renderer/extensions...
chrome/renderer/extensions/chrome_v8_context.cc:158: return
ExtensionAPI::GetSharedInstance()->IsPartAvailableToContext(
On 2013/05/01 20:47:04, kalman wrote:
> On 2013/05/01 02:51:47, cduvall wrote:
> > On 2013/04/27 01:02:43, kalman wrote:
> > > seems like this might be a dangerous oversimplification
> > 
> > Maybe it would be better to rename GetPossibleAvailabilityForContext().
> 
> Ah yeah good point. Something even more explicit perhaps, like
> IsAnyPartAvailableToContext()? IsAnyComponent... IsAnyFunctionEventOrProperty
> (kidding).. IsAnyFeatureAvailableToContext (mm sounding better)... eh,
> something. But it should at least start with Is.

Done.

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
File chrome/renderer/resources/extensions/binding.js (right):

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:262:
addUnprivilegedAccessGetter(mod, functionDef.name);
On 2013/05/01 20:47:04, kalman wrote:
> Just leave it blank. No need for the unprivileged thing.
> 
> though I think we'll want to fix 234311 and apply it here somehow. ugh. I
can't
> think of any APIs that would benefit from it yet, though.

Done.

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:325: var options =
eventDef.options || {};
On 2013/05/01 20:47:04, kalman wrote:
> might as well define options after this block.

Done.

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:327:
addUnprivilegedAccessGetter(mod, eventDef.name);
On 2013/05/01 20:47:04, kalman wrote:
> ditto - no unprivileged access getter, just leave it blank.
> 
> In fact, let's just delete addUnprivilegedAccessGetter entirely.

Done.

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:359:
addUnprivilegedAccessGetter(mod, propertyName);
On 2013/05/01 20:47:04, kalman wrote:
> ditto

Done.

https://codereview.chromium.org/14494013/diff/10001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:412:
(hasItemOfType(schema.events) || hasItemOfType(schema.functions))) {
On 2013/05/01 20:47:04, kalman wrote:
> a simpler test may be to just check whether the module object is empty. You
can
> use Object.keys(mod).length == 0.

Done.

[not at google - send to devlin, 2013-05-02 00:19:14]

code lgtm, but is there a test you can augment, or add, or make sure it's
tested? I mean like
- hosted apps don't get access to runtime
- content scripts don't get access to the privileged things in runtime

[commit-bot: I haz the power, 2013-05-02 00:40:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cduvall@chromium.org/14494013/31001

[commit-bot: I haz the power, 2013-05-02 03:08:08]

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

[commit-bot: I haz the power, 2013-05-02 03:11:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cduvall@chromium.org/14494013/31001

[cduvall, 2013-05-03 02:10:02]

Ben: could you take another look at this, I had to fix Stubs to inspect
_api_features.json, which required adding a bit of code.

[cduvall, 2013-05-03 02:14:02]

https://codereview.chromium.org/14494013/diff/56001/chrome/common/extensions/...
File chrome/common/extensions/api/events.json (right):

https://codereview.chromium.org/14494013/diff/56001/chrome/common/extensions/...
chrome/common/extensions/api/events.json:11: "check_unprivileged": true,
This is just necessary while these APIs have not been converted to features.
This is needed because bindings.js needs to check APIs that specify
"unprivileged" in the schema differently than the features APIs.

[not at google - send to devlin, 2013-05-03 04:58:14]

https://codereview.chromium.org/14494013/diff/56001/chrome/renderer/extension...
File chrome/renderer/extensions/dispatcher.cc (right):

https://codereview.chromium.org/14494013/diff/56001/chrome/renderer/extension...
chrome/renderer/extensions/dispatcher.cc:133:
IDR_EXTENSION_API_FEATURES).as_string());
do you need to write it back to a string again? you could turn it into a
v8::Object and do it that way, v8valueconverter.

https://codereview.chromium.org/14494013/diff/56001/chrome/renderer/resources...
File chrome/renderer/resources/extensions/binding.js (right):

https://codereview.chromium.org/14494013/diff/56001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:205: var checkUnprivileged =
schema['check_unprivileged'];
to answer your question in IRC - yeah let's fix this stuff up in a follow-up.

however, for now, can you just search over the whole API object looking for an
object with an "unprivileged" property? It's a bit inefficient, but, not really
that bad, less than total work. and it's evaluated lazily anyway.

https://codereview.chromium.org/14494013/diff/56001/chrome/test/data/extensio...
File chrome/test/data/extensions/api_test/stubs/content_script.js (right):

https://codereview.chromium.org/14494013/diff/56001/chrome/test/data/extensio...
chrome/test/data/extensions/api_test/stubs/content_script.js:17: var apiFeatures
= JSON.parse(chrome.test.getApiFeatures());
see comment in dispatcher.cc about just return this as a v8 object.

https://codereview.chromium.org/14494013/diff/56001/chrome/test/data/extensio...
chrome/test/data/extensions/api_test/stubs/content_script.js:18: function
IsUnprivileged(namespace, path) {
JS style is isUnprivileged not IsUnprivileged, for some reason. Though
isAvailableToContentScripts might be more descriptive - the unprivileged
terminology comes from a time before we distinguished between contexts in this
way.

[not at google - send to devlin, 2013-05-03 04:58:32]

lgtm again after those changes

[cduvall, 2013-05-07 23:44:28]

https://codereview.chromium.org/14494013/diff/56001/chrome/renderer/extension...
File chrome/renderer/extensions/dispatcher.cc (right):

https://codereview.chromium.org/14494013/diff/56001/chrome/renderer/extension...
chrome/renderer/extensions/dispatcher.cc:133:
IDR_EXTENSION_API_FEATURES).as_string());
On 2013/05/03 04:58:14, kalman wrote:
> do you need to write it back to a string again? you could turn it into a
> v8::Object and do it that way, v8valueconverter.

Done.

https://codereview.chromium.org/14494013/diff/56001/chrome/test/data/extensio...
File chrome/test/data/extensions/api_test/stubs/content_script.js (right):

https://codereview.chromium.org/14494013/diff/56001/chrome/test/data/extensio...
chrome/test/data/extensions/api_test/stubs/content_script.js:17: var apiFeatures
= JSON.parse(chrome.test.getApiFeatures());
On 2013/05/03 04:58:14, kalman wrote:
> see comment in dispatcher.cc about just return this as a v8 object.

Done.

https://codereview.chromium.org/14494013/diff/56001/chrome/test/data/extensio...
chrome/test/data/extensions/api_test/stubs/content_script.js:18: function
IsUnprivileged(namespace, path) {
On 2013/05/03 04:58:14, kalman wrote:
> JS style is isUnprivileged not IsUnprivileged, for some reason. Though
> isAvailableToContentScripts might be more descriptive - the unprivileged
> terminology comes from a time before we distinguished between contexts in this
> way.

Done.

[commit-bot: I haz the power, 2013-05-07 23:44:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cduvall@chromium.org/14494013/72001

[commit-bot: I haz the power, 2013-05-08 14:25:53]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-08 14:27:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cduvall@chromium.org/14494013/89002

[commit-bot: I haz the power, 2013-05-09 06:51:55]

Change committed as 199155