Extracting page shingle hashes for similarity detection.

Extracting page shingle hashes for similarity detection.

BUG=

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

[mattm, 2014-05-06 01:00:14]

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:252: }
each of the cases here duplicates the code of the previous case.

You could avoid that by doing:

append word
if (len == shingle_size) {
 calculate & insert shingle
 pop first word
}

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:269: if
(shingle_hashes_->size() >= max_hashes_per_page_) {
I'd probably more comfortable with:
if (word_lower.empty())

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.h (right):

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.h:61: size_t
max_hashes_per_page,
Also move this in argument list next to shingle_size since they are related?

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.h:61: size_t
max_hashes_per_page,
I would suggest max_shingles_per_page for this (and other similar names like in
the protobuf, member variables, etc) to make it less ambiguous.

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc
(right):

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc:253:
// we can correctly separate terms in languages that don't use spaces.
Seems we should also have a similar test with enough words that tests shingles
are generated properly for such languages also.

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc:273:
// since its size is too large.
what do you mean by size is too large? That you don't want to put all the
calculations for the expected shingles here?
Seems like these cases would be important to test..

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/scorer_unittest.cc (right):

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/scorer_unittest.cc:59: model_.set_shingle_size(4);
maybe use non-default value for this test?

[zysxqn, 2014-05-06 20:56:57]

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:252: }
On 2014/05/06 01:00:14, mattm wrote:
> each of the cases here duplicates the code of the previous case.
> 
> You could avoid that by doing:
> 
> append word
> if (len == shingle_size) {
>  calculate & insert shingle
>  pop first word
> }

Done.

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:269: if
(shingle_hashes_->size() >= max_hashes_per_page_) {
On 2014/05/06 01:00:14, mattm wrote:
> I'd probably more comfortable with:
> if (word_lower.empty())

Done.

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.h (right):

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.h:61: size_t
max_hashes_per_page,
On 2014/05/06 01:00:14, mattm wrote:
> Also move this in argument list next to shingle_size since they are related?

Done.

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.h:61: size_t
max_hashes_per_page,
On 2014/05/06 01:00:14, mattm wrote:
> Also move this in argument list next to shingle_size since they are related?

Done.

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc
(right):

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc:253:
// we can correctly separate terms in languages that don't use spaces.
On 2014/05/06 01:00:14, mattm wrote:
> Seems we should also have a similar test with enough words that tests shingles
> are generated properly for such languages also.

Done.

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc:273:
// since its size is too large.
On 2014/05/06 01:00:14, mattm wrote:
> what do you mean by size is too large? That you don't want to put all the
> calculations for the expected shingles here?
> Seems like these cases would be important to test..

Done.

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/scorer_unittest.cc (right):

https://codereview.chromium.org/268673007/diff/30001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/scorer_unittest.cc:59: model_.set_shingle_size(4);
On 2014/05/06 01:00:14, mattm wrote:
> maybe use non-default value for this test?

Done.

[noelutz, 2014-05-06 21:40:18]

Nice.  A few comments.
noe.

https://codereview.chromium.org/268673007/diff/50001/chrome/common/safe_brows...
File chrome/common/safe_browsing/client_model.proto (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/common/safe_brows...
chrome/common/safe_browsing/client_model.proto:93: optional int32
max_shingles_per_page = 9 [default = 200];
Don't forget to add these fields on hte server as well.

https://codereview.chromium.org/268673007/diff/50001/chrome/common/safe_brows...
File chrome/common/safe_browsing/csd.proto (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/common/safe_brows...
chrome/common/safe_browsing/csd.proto:73: repeated uint32 shingle_hashes = 12;
don't forget to mark these as packed as well.  Otherwise, we won't be able to
parse them on the server side.

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc:96:
model.set_max_shingles_per_page(200);
Is this necessary since you're setting these as default values?

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:230: if
(shingle_hashes_->size() < max_shingles_per_page_) {
I think we want to keep the min 200 shingles so that code isn't quite right. 
You should always add the new shingle and then remove the largest one if we have
more than max_shingles_per_page_ shingles in the set.

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.h (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.h:80: //
CancelPendingExtraction() is called.
nit: mention ownership of shingle_hashes as well?

[zysxqn, 2014-05-07 19:29:18]

https://codereview.chromium.org/268673007/diff/50001/chrome/common/safe_brows...
File chrome/common/safe_browsing/client_model.proto (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/common/safe_brows...
chrome/common/safe_browsing/client_model.proto:93: optional int32
max_shingles_per_page = 9 [default = 200];
On 2014/05/06 21:40:18, noelutz wrote:
> Don't forget to add these fields on hte server as well.

Acknowledged.

https://codereview.chromium.org/268673007/diff/50001/chrome/common/safe_brows...
File chrome/common/safe_browsing/csd.proto (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/common/safe_brows...
chrome/common/safe_browsing/csd.proto:73: repeated uint32 shingle_hashes = 12;
On 2014/05/06 21:40:18, noelutz wrote:
> don't forget to mark these as packed as well.  Otherwise, we won't be able to
> parse them on the server side.

Done.

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc:96:
model.set_max_shingles_per_page(200);
On 2014/05/06 21:40:18, noelutz wrote:
> Is this necessary since you're setting these as default values?

Prefer to set a non-default value than removing it since this will catch some
minor errors such as name inconsistency.

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:230: if
(shingle_hashes_->size() < max_shingles_per_page_) {
On 2014/05/06 21:40:18, noelutz wrote:
> I think we want to keep the min 200 shingles so that code isn't quite right. 
> You should always add the new shingle and then remove the largest one if we
have
> more than max_shingles_per_page_ shingles in the set.

I know on the server side experiment we use min 200 hashes. But for the sake of
computing similarity I don't see any problem with keeping first 200 hashes, as
long as the extracting logic is the same among all pages. Am I miss anything?

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.h (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.h:80: //
CancelPendingExtraction() is called.
On 2014/05/06 21:40:18, noelutz wrote:
> nit: mention ownership of shingle_hashes as well?

Done.

[zysxqn, 2014-05-09 21:34:24]

Please have another look. Thanks!

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:230: if
(shingle_hashes_->size() < max_shingles_per_page_) {
On 2014/05/06 21:40:18, noelutz wrote:
> I think we want to keep the min 200 shingles so that code isn't quite right. 
> You should always add the new shingle and then remove the largest one if we
have
> more than max_shingles_per_page_ shingles in the set.

Done.

[mattm, 2014-05-09 23:28:10]

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:230: if
(shingle_hashes_->size() < max_shingles_per_page_) {
On 2014/05/09 21:34:24, zysxqn wrote:
> On 2014/05/06 21:40:18, noelutz wrote:
> > I think we want to keep the min 200 shingles so that code isn't quite right.

> > You should always add the new shingle and then remove the largest one if we
> have
> > more than max_shingles_per_page_ shingles in the set.
> 
> Done.


Have you tested if doing it for the whole text has any notable performance
impact? (Not just the hashing, but the extra lowercasing and utf conversions
which are called expensive below.)

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:241:
shingle_hashes_->erase(--it);
does shingle_hashes_->erase(--shingle_hashes_->end()); work?

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:248: // which
are relatively expensive.
Since the conversion and lowercasing is always done, is this cache useful
anymore?

[noelutz, 2014-05-10 01:01:19]

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:230: if
(shingle_hashes_->size() < max_shingles_per_page_) {
On 2014/05/09 23:28:10, mattm wrote:
> On 2014/05/09 21:34:24, zysxqn wrote:
> > On 2014/05/06 21:40:18, noelutz wrote:
> > > I think we want to keep the min 200 shingles so that code isn't quite
right.
> 
> > > You should always add the new shingle and then remove the largest one if
we
> > have
> > > more than max_shingles_per_page_ shingles in the set.
> > 
> > Done.
> 
> 
> Have you tested if doing it for the whole text has any notable performance
> impact? (Not just the hashing, but the extra lowercasing and utf conversions
> which are called expensive below.)

+1 on performance testing.  Could you add a histogram measuring how much time we
spend doing that?

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:241:
shingle_hashes_->erase(--it);
On 2014/05/09 23:28:10, mattm wrote:
> does shingle_hashes_->erase(--shingle_hashes_->end()); work?

Or shingle_hashes_->erase(shingle_hashes_->rbegin());

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:248: // which
are relatively expensive.
On 2014/05/09 23:28:10, mattm wrote:
> Since the conversion and lowercasing is always done, is this cache useful
> anymore?

It saves us from unnecessary hashing (line 257), no?

[zysxqn, 2014-05-12 17:43:20]

On 2014/05/10 01:01:19, noelutz wrote:
>
https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
> File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):
> 
>
https://codereview.chromium.org/268673007/diff/50001/chrome/renderer/safe_bro...
> chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:230: if
> (shingle_hashes_->size() < max_shingles_per_page_) {
> On 2014/05/09 23:28:10, mattm wrote:
> > On 2014/05/09 21:34:24, zysxqn wrote:
> > > On 2014/05/06 21:40:18, noelutz wrote:
> > > > I think we want to keep the min 200 shingles so that code isn't quite
> right.
> > 
> > > > You should always add the new shingle and then remove the largest one if
> we
> > > have
> > > > more than max_shingles_per_page_ shingles in the set.
> > > 
> > > Done.
> > 
> > 
> > Have you tested if doing it for the whole text has any notable performance
> > impact? (Not just the hashing, but the extra lowercasing and utf conversions
> > which are called expensive below.)
> 
> +1 on performance testing.  Could you add a histogram measuring how much time
we
> spend doing that?

  Given that it's not blocking the main rendering process and we already have a
timeout mechanism for too long extraction. Wouldn't it be much easier to just
track the existing timeout event in histogram? If it turns out to increase a lot
after checking in this CL we know it's because of this change and we can
consider something else.
> 
>
https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
> File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):
> 
>
https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
> chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:241:
> shingle_hashes_->erase(--it);
> On 2014/05/09 23:28:10, mattm wrote:
> > does shingle_hashes_->erase(--shingle_hashes_->end()); work?
> 
> Or shingle_hashes_->erase(shingle_hashes_->rbegin());
> 
>
https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
> chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:248: // which
> are relatively expensive.
> On 2014/05/09 23:28:10, mattm wrote:
> > Since the conversion and lowercasing is always done, is this cache useful
> > anymore?
> 
> It saves us from unnecessary hashing (line 257), no?

[zysxqn, 2014-05-12 17:43:56]

No changes of the code. But replies to the previous comments. Please have
another look.

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:241:
shingle_hashes_->erase(--it);
On 2014/05/10 01:01:20, noelutz wrote:
> On 2014/05/09 23:28:10, mattm wrote:
> > does shingle_hashes_->erase(--shingle_hashes_->end()); work?
> 
> Or shingle_hashes_->erase(shingle_hashes_->rbegin());

Unfortunately neither works since set doesn't support random access iterator
(it's a bidirectional iterator).

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:241:
shingle_hashes_->erase(--it);
On 2014/05/09 23:28:10, mattm wrote:
> does shingle_hashes_->erase(--shingle_hashes_->end()); work?

Nope.. See the reply below.

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:248: // which
are relatively expensive.
On 2014/05/09 23:28:10, mattm wrote:
> Since the conversion and lowercasing is always done, is this cache useful
> anymore?

We can still prevent unnecessary hashing on the individual word below.

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:248: // which
are relatively expensive.
On 2014/05/10 01:01:20, noelutz wrote:
> On 2014/05/09 23:28:10, mattm wrote:
> > Since the conversion and lowercasing is always done, is this cache useful
> > anymore?
> 
> It saves us from unnecessary hashing (line 257), no?

Acknowledged.

[noelutz, 2014-05-12 18:01:29]

lgtm

Please wait for Matt's OK was well.

[mattm, 2014-05-12 20:08:14]

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:248: // which
are relatively expensive.
On 2014/05/12 17:43:57, zysxqn wrote:
> On 2014/05/09 23:28:10, mattm wrote:
> > Since the conversion and lowercasing is always done, is this cache useful
> > anymore?
> 
> We can still prevent unnecessary hashing on the individual word below.

Doing the lookup in negative_word_cache_ also requires hashing the string, so
that only seems advantageous if murmurhash3 is a lot slower than the STL hash
function.

[zysxqn, 2014-05-12 21:09:33]

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:248: // which
are relatively expensive.
On 2014/05/12 20:08:14, mattm wrote:
> On 2014/05/12 17:43:57, zysxqn wrote:
> > On 2014/05/09 23:28:10, mattm wrote:
> > > Since the conversion and lowercasing is always done, is this cache useful
> > > anymore?
> > 
> > We can still prevent unnecessary hashing on the individual word below.
> 
> Doing the lookup in negative_word_cache_ also requires hashing the string, so
> that only seems advantageous if murmurhash3 is a lot slower than the STL hash
> function.

Murmurhash3 is a relatively fast hash function so I'm fine with removing the
caching. Noe, what's your opinion on this?

[noelutz, 2014-05-12 21:51:08]

lgtm

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:248: // which
are relatively expensive.
On 2014/05/12 21:09:34, zysxqn wrote:
> On 2014/05/12 20:08:14, mattm wrote:
> > On 2014/05/12 17:43:57, zysxqn wrote:
> > > On 2014/05/09 23:28:10, mattm wrote:
> > > > Since the conversion and lowercasing is always done, is this cache
useful
> > > > anymore?
> > > 
> > > We can still prevent unnecessary hashing on the individual word below.
> > 
> > Doing the lookup in negative_word_cache_ also requires hashing the string,
so
> > that only seems advantageous if murmurhash3 is a lot slower than the STL
hash
> > function.
> 
> Murmurhash3 is a relatively fast hash function so I'm fine with removing the
> caching. Noe, what's your opinion on this?

I'm fine with removing the cache.  It looks like murmurhash is really fast.

[zysxqn, 2014-05-12 22:08:56]

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
File chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc (right):

https://codereview.chromium.org/268673007/diff/110001/chrome/renderer/safe_br...
chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc:248: // which
are relatively expensive.
On 2014/05/12 20:08:14, mattm wrote:
> On 2014/05/12 17:43:57, zysxqn wrote:
> > On 2014/05/09 23:28:10, mattm wrote:
> > > Since the conversion and lowercasing is always done, is this cache useful
> > > anymore?
> > 
> > We can still prevent unnecessary hashing on the individual word below.
> 
> Doing the lookup in negative_word_cache_ also requires hashing the string, so
> that only seems advantageous if murmurhash3 is a lot slower than the STL hash
> function.

Removed.

[mattm, 2014-05-12 22:18:15]

lgtm

[zysxqn, 2014-05-12 22:23:13]

The CQ bit was checked by zysxqn@google.com

[commit-bot: I haz the power, 2014-05-12 22:24:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/zysxqn@google.com/268673007/130001

[commit-bot: I haz the power, 2014-05-13 02:54:57]

Change committed as 269976