Disk cache: Clarify API contract and delete spurious delay.

Disk cache: Clarify API contract and delete spurious delay.

The Doom* contract is meant to follow conventions used for other browsing data
types, like the cookie monster. This CL clarifies the time comparisons to use.

It also removes an extra delay introduced in r219969.

BUG=330926
TEST=current

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

[rvargas (doing something else), 2014-03-18 17:53:39]

PTAL

[gavinp, 2014-03-21 11:20:39]

https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
File net/disk_cache/backend_unittest.cc (left):

https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
net/disk_cache/backend_unittest.cc:1607: AddDelay();
This delay is not spurious; for caches that use coarse timing (second resolution
for instance in simple cache), this delay keeps the first doom from erasing
entries created after time middle_end.

[rvargas (doing something else), 2014-03-21 18:14:41]

https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
File net/disk_cache/backend_unittest.cc (left):

https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
net/disk_cache/backend_unittest.cc:1607: AddDelay();
On 2014/03/21 11:20:39, gavinp wrote:
> This delay is not spurious; for caches that use coarse timing (second
resolution
> for instance in simple cache), this delay keeps the first doom from erasing
> entries created after time middle_end.

Could you expand on that?

The test as it is _without_ this delay is meant to make sure that the cache
doesn't delete entries created _after_ |middle_end|, as that is the only thing
visible by the caller. Failing the test without the delay still sounds like a
bug to me as the backend is not respecting the provided end time.

[gavinp, 2014-03-21 18:56:28]

On 2014/03/21 18:14:41, rvargas wrote:
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> File net/disk_cache/backend_unittest.cc (left):
> 
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> net/disk_cache/backend_unittest.cc:1607: AddDelay();
> On 2014/03/21 11:20:39, gavinp wrote:
> > This delay is not spurious; for caches that use coarse timing (second
> resolution
> > for instance in simple cache), this delay keeps the first doom from erasing
> > entries created after time middle_end.
> 
> Could you expand on that?
> 
> The test as it is _without_ this delay is meant to make sure that the cache
> doesn't delete entries created _after_ |middle_end|, as that is the only thing
> visible by the caller. Failing the test without the delay still sounds like a
> bug to me as the backend is not respecting the provided end time.

Sadly, a closed time interval will always cause entries created after middle_end
to be potentially deleted. The time can stay the same for a long time. The
simple cache, because it rounds time fairly coarsely, gets this even more.

However, the way forward is probably just to remove end_time from the API. Is
there any non-test use of this parameter?

[rvargas (doing something else), 2014-03-21 18:59:31]

On 2014/03/21 18:56:28, gavinp wrote:
> On 2014/03/21 18:14:41, rvargas wrote:
> >
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> > File net/disk_cache/backend_unittest.cc (left):
> > 
> >
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> > net/disk_cache/backend_unittest.cc:1607: AddDelay();
> > On 2014/03/21 11:20:39, gavinp wrote:
> > > This delay is not spurious; for caches that use coarse timing (second
> > resolution
> > > for instance in simple cache), this delay keeps the first doom from
erasing
> > > entries created after time middle_end.
> > 
> > Could you expand on that?
> > 
> > The test as it is _without_ this delay is meant to make sure that the cache
> > doesn't delete entries created _after_ |middle_end|, as that is the only
thing
> > visible by the caller. Failing the test without the delay still sounds like
a
> > bug to me as the backend is not respecting the provided end time.
> 
> Sadly, a closed time interval will always cause entries created after
middle_end
> to be potentially deleted. The time can stay the same for a long time. The

But the interface is not a closed time interval. It is open on the end time.

> simple cache, because it rounds time fairly coarsely, gets this even more.
> 
> However, the way forward is probably just to remove end_time from the API. Is
> there any non-test use of this parameter?

[gavinp, 2014-03-21 19:10:30]

On 2014/03/21 18:59:31, rvargas wrote:
> On 2014/03/21 18:56:28, gavinp wrote:
> > On 2014/03/21 18:14:41, rvargas wrote:
> > >
> >
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> > > File net/disk_cache/backend_unittest.cc (left):
> > > 
> > >
> >
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> > > net/disk_cache/backend_unittest.cc:1607: AddDelay();
> > > On 2014/03/21 11:20:39, gavinp wrote:
> > > > This delay is not spurious; for caches that use coarse timing (second
> > > resolution
> > > > for instance in simple cache), this delay keeps the first doom from
> erasing
> > > > entries created after time middle_end.
> > > 
> > > Could you expand on that?
> > > 
> > > The test as it is _without_ this delay is meant to make sure that the
cache
> > > doesn't delete entries created _after_ |middle_end|, as that is the only
> thing
> > > visible by the caller. Failing the test without the delay still sounds
like
> a
> > > bug to me as the backend is not respecting the provided end time.
> > 
> > Sadly, a closed time interval will always cause entries created after
> middle_end
> > to be potentially deleted. The time can stay the same for a long time. The
> 
> But the interface is not a closed time interval. It is open on the end time.
> 

When clocks are coarse, open intervals become another way of saying a closed
interval.

An open interval to time t, where t is coarsely measured so it could be anything
from t to t', must be really treated as a coarse interval to t', otherwise we
aren't respecting users private data correctly, we could not delete data they
wished to delete.

So, we have to treate end time as a closed interval.

But this is all moot, isn't it? Is there any non-test use of end_time? Why don't
we just change the API to remove this parameter, and all deletions will be from
start_time until the end of time?

> > simple cache, because it rounds time fairly coarsely, gets this even more.
> > 
> > However, the way forward is probably just to remove end_time from the API.
Is
> > there any non-test use of this parameter?

[rvargas (doing something else), 2014-03-21 19:48:40]

On 2014/03/21 19:10:30, gavinp wrote:
> On 2014/03/21 18:59:31, rvargas wrote:
> > On 2014/03/21 18:56:28, gavinp wrote:
> > > On 2014/03/21 18:14:41, rvargas wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> > > > File net/disk_cache/backend_unittest.cc (left):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> > > > net/disk_cache/backend_unittest.cc:1607: AddDelay();
> > > > On 2014/03/21 11:20:39, gavinp wrote:
> > > > > This delay is not spurious; for caches that use coarse timing (second
> > > > resolution
> > > > > for instance in simple cache), this delay keeps the first doom from
> > erasing
> > > > > entries created after time middle_end.
> > > > 
> > > > Could you expand on that?
> > > > 
> > > > The test as it is _without_ this delay is meant to make sure that the
> cache
> > > > doesn't delete entries created _after_ |middle_end|, as that is the only
> > thing
> > > > visible by the caller. Failing the test without the delay still sounds
> like
> > a
> > > > bug to me as the backend is not respecting the provided end time.
> > > 
> > > Sadly, a closed time interval will always cause entries created after
> > middle_end
> > > to be potentially deleted. The time can stay the same for a long time. The
> > 
> > But the interface is not a closed time interval. It is open on the end time.
> > 
> 
> When clocks are coarse, open intervals become another way of saying a closed
> interval.
> 
> An open interval to time t, where t is coarsely measured so it could be
anything
> from t to t', must be really treated as a coarse interval to t', otherwise we
> aren't respecting users private data correctly, we could not delete data they
> wished to delete.
> 
> So, we have to treate end time as a closed interval.
> 
> But this is all moot, isn't it? Is there any non-test use of end_time? Why
don't
> we just change the API to remove this parameter, and all deletions will be
from
> start_time until the end of time?
> 
> > > simple cache, because it rounds time fairly coarsely, gets this even more.
> > > 
> > > However, the way forward is probably just to remove end_time from the API.
> Is
> > > there any non-test use of this parameter?

In general, the expectation of the API is that if an entry is modified at
base::Time() t, calling any of the Doom methods with that t within a range (or
outside) will work. While I see your point about lack of resolution on the
implementation, that is not _really_ expected from the point of view of the API,
and we have exactly the same issue about not respecting the caller regarding the
start time. On a similar line, it is surprising that adding a delay of one unit
on the test (maybe 1ms) would fix things if the actual internal resolution of
the backend is 1 second.

On the other hand, _when_ the time is measured is not part of the actual
contract (it is valid to say, for instance, that times are actually updated only
when an entry is closed). So if something like that would fix the test without
adding a weird delay (weird because it doesn't follow the logic of the test),
I'm more than happy to fix it that way. I have to say that before opting for
disabling the test I looked at the code and it was not obvious to me why it
failed without the delay.

As for removing the API, I'm not opposed to that, as it has never been used and
it is very unlikely to be used... as long as the related APIs are also gone
(Cookies). I'd be happy to review that code.

[gavinp, 2014-03-26 15:17:05]

On 2014/03/21 19:48:40, rvargas wrote:
> On 2014/03/21 19:10:30, gavinp wrote:
> > On 2014/03/21 18:59:31, rvargas wrote:
> > > On 2014/03/21 18:56:28, gavinp wrote:
> > > > On 2014/03/21 18:14:41, rvargas wrote:
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> > > > > File net/disk_cache/backend_unittest.cc (left):
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> > > > > net/disk_cache/backend_unittest.cc:1607: AddDelay();
> > > > > On 2014/03/21 11:20:39, gavinp wrote:
> > > > > > This delay is not spurious; for caches that use coarse timing
(second
> > > > > resolution
> > > > > > for instance in simple cache), this delay keeps the first doom from
> > > erasing
> > > > > > entries created after time middle_end.
> > > > > 
> > > > > Could you expand on that?
> > > > > 
> > > > > The test as it is _without_ this delay is meant to make sure that the
> > cache
> > > > > doesn't delete entries created _after_ |middle_end|, as that is the
only
> > > thing
> > > > > visible by the caller. Failing the test without the delay still sounds
> > like
> > > a
> > > > > bug to me as the backend is not respecting the provided end time.
> > > > 
> > > > Sadly, a closed time interval will always cause entries created after
> > > middle_end
> > > > to be potentially deleted. The time can stay the same for a long time.
The
> > > 
> > > But the interface is not a closed time interval. It is open on the end
time.
> > > 
> > 
> > When clocks are coarse, open intervals become another way of saying a closed
> > interval.
> > 
> > An open interval to time t, where t is coarsely measured so it could be
> anything
> > from t to t', must be really treated as a coarse interval to t', otherwise
we
> > aren't respecting users private data correctly, we could not delete data
they
> > wished to delete.
> > 
> > So, we have to treate end time as a closed interval.
> > 
> > But this is all moot, isn't it? Is there any non-test use of end_time? Why
> don't
> > we just change the API to remove this parameter, and all deletions will be
> from
> > start_time until the end of time?
> > 
> > > > simple cache, because it rounds time fairly coarsely, gets this even
more.
> > > > 
> > > > However, the way forward is probably just to remove end_time from the
API.
> > Is
> > > > there any non-test use of this parameter?
> 
> In general, the expectation of the API is that if an entry is modified at
> base::Time() t, calling any of the Doom methods with that t within a range (or
> outside) will work. While I see your point about lack of resolution on the
> implementation, that is not _really_ expected from the point of view of the
API,
> and we have exactly the same issue about not respecting the caller regarding
the
> start time.

So again, this is all moot, since this code is dead except in tests.

Further, even if it wasn't dead code, that expectation of the API isn't possible
to implement on any computer with a coarse clock, for the reasons outlined in
comment #6. This is true for any clock that's coarse, base::Time's clock is
coarse as well. The choice is between respecting user intent/privacy and
implementing a conceptually broken API model... Well, I think the choice is
clear.

> On a similar line, it is surprising that adding a delay of one unit
> on the test (maybe 1ms) would fix things if the actual internal resolution of
> the backend is 1 second.

That would be surprising, but that's not what's happening. The implementation of
AddDelay waits long enough to ensure that the clock ticks appropriately for all
types of cache backend.

> On the other hand, _when_ the time is measured is not part of the actual
> contract (it is valid to say, for instance, that times are actually updated
only
> when an entry is closed). So if something like that would fix the test without
> adding a weird delay (weird because it doesn't follow the logic of the test),
> I'm more than happy to fix it that way. I have to say that before opting for
> disabling the test I looked at the code and it was not obvious to me why it
> failed without the delay.

Can you expand on this? I don't understand it.

> As for removing the API, I'm not opposed to that, as it has never been used
and
> it is very unlikely to be used... as long as the related APIs are also gone
> (Cookies). I'd be happy to review that code.

I would be also happy to review it. But I'm not going to get to it soon. But in
the meantime, I am not enthusiastic about updates to dead code.

[rvargas (doing something else), 2014-03-26 22:44:26]

On 2014/03/26 15:17:05, gavinp wrote:
> On 2014/03/21 19:48:40, rvargas wrote:
> > On 2014/03/21 19:10:30, gavinp wrote:
> > > On 2014/03/21 18:59:31, rvargas wrote:
> > > > On 2014/03/21 18:56:28, gavinp wrote:
> > > > > On 2014/03/21 18:14:41, rvargas wrote:
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> > > > > > File net/disk_cache/backend_unittest.cc (left):
> > > > > > 
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/118123004/diff/100001/net/disk_cache/backend_...
> > > > > > net/disk_cache/backend_unittest.cc:1607: AddDelay();
> > > > > > On 2014/03/21 11:20:39, gavinp wrote:
> > > > > > > This delay is not spurious; for caches that use coarse timing
> (second
> > > > > > resolution
> > > > > > > for instance in simple cache), this delay keeps the first doom
from
> > > > erasing
> > > > > > > entries created after time middle_end.
> > > > > > 
> > > > > > Could you expand on that?
> > > > > > 
> > > > > > The test as it is _without_ this delay is meant to make sure that
the
> > > cache
> > > > > > doesn't delete entries created _after_ |middle_end|, as that is the
> only
> > > > thing
> > > > > > visible by the caller. Failing the test without the delay still
sounds
> > > like
> > > > a
> > > > > > bug to me as the backend is not respecting the provided end time.
> > > > > 
> > > > > Sadly, a closed time interval will always cause entries created after
> > > > middle_end
> > > > > to be potentially deleted. The time can stay the same for a long time.
> The
> > > > 
> > > > But the interface is not a closed time interval. It is open on the end
> time.
> > > > 
> > > 
> > > When clocks are coarse, open intervals become another way of saying a
closed
> > > interval.
> > > 
> > > An open interval to time t, where t is coarsely measured so it could be
> > anything
> > > from t to t', must be really treated as a coarse interval to t', otherwise
> we
> > > aren't respecting users private data correctly, we could not delete data
> they
> > > wished to delete.
> > > 
> > > So, we have to treate end time as a closed interval.
> > > 
> > > But this is all moot, isn't it? Is there any non-test use of end_time? Why
> > don't
> > > we just change the API to remove this parameter, and all deletions will be
> > from
> > > start_time until the end of time?
> > > 
> > > > > simple cache, because it rounds time fairly coarsely, gets this even
> more.
> > > > > 
> > > > > However, the way forward is probably just to remove end_time from the
> API.
> > > Is
> > > > > there any non-test use of this parameter?
> > 
> > In general, the expectation of the API is that if an entry is modified at
> > base::Time() t, calling any of the Doom methods with that t within a range
(or
> > outside) will work. While I see your point about lack of resolution on the
> > implementation, that is not _really_ expected from the point of view of the
> API,
> > and we have exactly the same issue about not respecting the caller regarding
> the
> > start time.
> 
> So again, this is all moot, since this code is dead except in tests.
> 
> Further, even if it wasn't dead code, that expectation of the API isn't
possible
> to implement on any computer with a coarse clock, for the reasons outlined in
> comment #6. This is true for any clock that's coarse, base::Time's clock is
> coarse as well. The choice is between respecting user intent/privacy and
> implementing a conceptually broken API model... Well, I think the choice is
> clear.

what now? Now that we are going into esoteric territory, "human" time is a
discrete concept so that description of "coarse clock" is at best funny. I
should be able to say "delete everything with a timestamp lower than this"
regardless of how that timestamp was calculated. If you want to argue that the
simple cache cannot make any guarantee about timestamps that's another story.


> 
> > On a similar line, it is surprising that adding a delay of one unit
> > on the test (maybe 1ms) would fix things if the actual internal resolution
of
> > the backend is 1 second.
> 
> That would be surprising, but that's not what's happening. The implementation
of
> AddDelay waits long enough to ensure that the clock ticks appropriately for
all
> types of cache backend.

I guess I have some more code changes that I missed...

> 
> > On the other hand, _when_ the time is measured is not part of the actual
> > contract (it is valid to say, for instance, that times are actually updated
> only
> > when an entry is closed). So if something like that would fix the test
without
> > adding a weird delay (weird because it doesn't follow the logic of the
test),
> > I'm more than happy to fix it that way. I have to say that before opting for
> > disabling the test I looked at the code and it was not obvious to me why it
> > failed without the delay.
> 
> Can you expand on this? I don't understand it.

I'm just pointing out that even though the Doom API is specific about how an
implementation should consider the timestamps that it receives, there is ample
room for implementations to decide what is the "right" timestamp value to use as
last_use and last_modified.

It is totally valid to grab that time everytime a read or write happens, but it
seems also fine to ignore intermediate operations and only grab the time say at
open and close. Such backend would most likely need some extra constrain in the
tests to make sure that the test "sees" the right events.

On the other hand, this extra delay violates the logic of the test (and the API)
as there is already a delay between "third" and "fouth", and the API dictates
that events after that delay should be grouped with "fourth"


> 
> > As for removing the API, I'm not opposed to that, as it has never been used
> and
> > it is very unlikely to be used... as long as the related APIs are also gone
> > (Cookies). I'd be happy to review that code.
> 
> I would be also happy to review it. But I'm not going to get to it soon. But
in
> the meantime, I am not enthusiastic about updates to dead code.

Well... this is the net API. You just changed the test that verified the
contract when adding the code (bad) and ignored my comment on that CL (bad) and
ignored the bug that I filed three months ago. If you are saying that the simple
cache doesn't care about this, fine (I give you that is not needed for chrome),
but the logical consequence is to disable the test for that backend, which is
exactly what I am doing.

[gavinp, 2014-03-31 18:38:56]

I'm really sorry, I can't really usefully continue this review.

Any attempt to delete a range of times is going to have to make a decision about
which error to make; clocks are coarse, and the decision is going to be about
whether to be overly inclusive or overly exclusive at each boundary. The concept
of closed and open are somewhat dangerous to user intention because time, as
reported by computers, is coarse; yes it's coarse in the simple cache especially
(because it's one second coarse), but it's coarse on all other platforms.

You yourself have commented in the past on many reviews about how long it is
that base::Time() can continue to return the same value.

Given the decision about respecting this open/closed convention, and making the
implementation that respects user privacy and intention, we chose user privacy
and intention. That was not done ignoring your comments, it was made considering
them and weighing them against the balance. The decision was made to not respect
that API convention because it is potentially harmful to our users. Yes, this
harm is most likely around the simple cache because it's very coarse. *shrug*

But, all that having been said, it's not actually even important, because this
API is test only. The useful change would be to just remove this end_time
parameter, which is test only and really just serves to add logic to every cache
backend and the cookie manager that nobody ever cares about. I guess disabling
the test on simple cache is arguably a step towards that, since that test checks
the begin time logic, I would rather see it stay in so we have coverage...

This change does not lgtm. I'm not enthusiastic about test only changes, and I'm
not enthusiastic about esoteric arguments about APIs that have these kinds of
subtle snakes lying in their bushes.

[gavinp, 2014-03-31 20:25:57]

And here's a TL;DR:

Consider this code:

     new_entry = disk_cache->CreateEntry(stuff);
     new_entry->WriteData(more stuff);
     now = base::Time::Now();
     disk_cache->DoomEntriesBetween(now - base::TimeDelta::FromHours(1), now);

The proposed fix and API to obey requires that new_entry is often not deleted by
this DoomEntriesBetween call. That's really confusing and not particularly
respectful of user privacy; the API is the problem, not the failure of one
backend to implement this particular "guarantee."

On 2014/03/31 18:38:56, gavinp wrote:
> I'm really sorry, I can't really usefully continue this review.
> 
> Any attempt to delete a range of times is going to have to make a decision
about
> which error to make; clocks are coarse, and the decision is going to be about
> whether to be overly inclusive or overly exclusive at each boundary. The
concept
> of closed and open are somewhat dangerous to user intention because time, as
> reported by computers, is coarse; yes it's coarse in the simple cache
especially
> (because it's one second coarse), but it's coarse on all other platforms.
> 
> You yourself have commented in the past on many reviews about how long it is
> that base::Time() can continue to return the same value.
> 
> Given the decision about respecting this open/closed convention, and making
the
> implementation that respects user privacy and intention, we chose user privacy
> and intention. That was not done ignoring your comments, it was made
considering
> them and weighing them against the balance. The decision was made to not
respect
> that API convention because it is potentially harmful to our users. Yes, this
> harm is most likely around the simple cache because it's very coarse. *shrug*
> 
> But, all that having been said, it's not actually even important, because this
> API is test only. The useful change would be to just remove this end_time
> parameter, which is test only and really just serves to add logic to every
cache
> backend and the cookie manager that nobody ever cares about. I guess disabling
> the test on simple cache is arguably a step towards that, since that test
checks
> the begin time logic, I would rather see it stay in so we have coverage...
> 
> This change does not lgtm. I'm not enthusiastic about test only changes, and
I'm
> not enthusiastic about esoteric arguments about APIs that have these kinds of
> subtle snakes lying in their bushes.

[rvargas (doing something else), 2014-03-31 22:12:57]

On 2014/03/31 20:25:57, gavinp wrote:
> And here's a TL;DR:
> 
> Consider this code:
> 
>      new_entry = disk_cache->CreateEntry(stuff);
>      new_entry->WriteData(more stuff);
>      now = base::Time::Now();
>      disk_cache->DoomEntriesBetween(now - base::TimeDelta::FromHours(1), now);
> 
> The proposed fix and API to obey requires that new_entry is often not deleted
by
> this DoomEntriesBetween call. That's really confusing and not particularly
> respectful of user privacy; the API is the problem, not the failure of one
> backend to implement this particular "guarantee."
> 

I'm not proposing any new API contract. I'm just formalizing the _current_ API
contract, and fixing a test to respect the current API contract. As far as I'm
concerned, your CL actually broke the contract and it is a regression.

If you want to change the contract, I'm happy to review.

If you don't want to fix the test for the simple cache, I'm happy to let it be
that way (test disabled, as I'm making it here).

Just because you failed to address the concern over your CL when raised is not a
justification to consider that CL as without issues.

[rvargas (doing something else), 2014-05-06 23:29:13]

I created another bug and added a new bug reference to the disabled test, as we
discussed.

[gavinp, 2014-05-15 00:49:00]

lgtm. Sorry for the delay; have been at Blinkon in Zurich.

[rvargas (doing something else), 2014-05-15 01:36:44]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-05-15 01:37:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rvargas@chromium.org/118123004/130001

[commit-bot: I haz the power, 2014-05-15 03:24:12]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[commit-bot: I haz the power, 2014-05-15 03:28:03]

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

[commit-bot: I haz the power, 2014-05-15 03:28:04]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[rvargas (doing something else), 2014-05-15 19:07:53]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-05-15 19:08:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rvargas@chromium.org/118123004/130001

[commit-bot: I haz the power, 2014-05-15 19:43:54]

Change committed as 270784