Stop using CertStore which is not compatible with PlzNavigate.

Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with
PlzNavigate the process ID isn't known when the resource loading is
happening.

BUG=598073, 504347
TBR=tsepez
NOTRY=true

Committed: https://crrev.com/8ae7cadae586d05e906fa581702e77c32080c4e7
Cr-Commit-Position: refs/heads/master@{#417444}

[jam, 2016-09-08 03:53:40]

Description was changed from

==========
Stop using CertStore which is not compatible with PlzNavigate.

BUG=598073, 504347
==========

to

==========
Stop using CertStore which is not compatible with PlzNavigate.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[jam, 2016-09-08 03:53:47]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 03:54:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 04:04:49]

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

[commit-bot: I haz the power, 2016-09-08 04:04:50]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[jam, 2016-09-08 04:25:29]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 04:25:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 04:36:56]

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

[commit-bot: I haz the power, 2016-09-08 04:36:57]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[jam, 2016-09-08 04:50:34]

Patchset #3 (id:40001) has been deleted

[jam, 2016-09-08 04:51:01]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 04:51:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 05:02:44]

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

[commit-bot: I haz the power, 2016-09-08 05:02:45]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[jam, 2016-09-08 05:25:20]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 05:25:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 05:52:30]

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

[commit-bot: I haz the power, 2016-09-08 05:52:31]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[jam, 2016-09-08 06:25:01]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 06:25:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-09-08 06:26:07]

Description was changed from

==========
Stop using CertStore which is not compatible with PlzNavigate.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with PlzNavigate
the process ID isn't known when the resource loading is happening.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-09-08 06:38:02]

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

[commit-bot: I haz the power, 2016-09-08 06:38:03]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[jam, 2016-09-08 06:45:13]

Patchset #5 (id:100001) has been deleted

[jam, 2016-09-08 06:45:14]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 06:45:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 07:19:15]

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

[commit-bot: I haz the power, 2016-09-08 07:19:16]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jam, 2016-09-08 14:35:36]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 14:35:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-09-08 15:08:42]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 15:08:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-09-08 15:19:47]

Patchset #5 (id:120001) has been deleted

[jam, 2016-09-08 15:21:03]

Patchset #5 (id:140001) has been deleted

[jam, 2016-09-08 15:46:11]

Description was changed from

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with PlzNavigate
the process ID isn't known when the resource loading is happening.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with PlzNavigate
the process ID isn't known when the resource loading is happening.

This fixes 82 failing browser tests.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[jam, 2016-09-08 15:48:14]

Description was changed from

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with PlzNavigate
the process ID isn't known when the resource loading is happening.

This fixes 82 failing browser tests.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with PlzNavigate
the process ID isn't known when the resource loading is happening.

This fixes 78 failing browser tests.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[jam, 2016-09-08 16:15:49]

Description was changed from

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with PlzNavigate
the process ID isn't known when the resource loading is happening.

This fixes 78 failing browser tests.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with PlzNavigate
the process ID isn't known when the resource loading is happening.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[jam, 2016-09-08 16:27:59]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 16:28:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-09-08 16:29:14]

jam@chromium.org changed reviewers:
+ eugenebut@chromium.org, felt@chromium.org, pfeldman@chromium.org

[jam, 2016-09-08 16:29:16]

eugenebut: ios/
pfeldman: third_party/WebKit/
felt: the rest

[commit-bot: I haz the power, 2016-09-08 16:32:03]

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

[commit-bot: I haz the power, 2016-09-08 16:32:07]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[felt, 2016-09-08 16:34:33]

felt@chromium.org changed reviewers:
+ rsleevi@chromium.org

[felt, 2016-09-08 16:34:53]

+rsleevi; someone on the networking team should also review

[Eugene But (OOO till 7-30), 2016-09-08 16:57:46]

ios lgtm. Is there a reason why you use |if (!!certificate.get())| instead of
|if (certificate)|? scoped_refptr implements |operator bool()|, so |if
(certificate)| should work fine.

https://codereview.chromium.org/2327433002/diff/180001/ios/chrome/browser/ssl...
File ios/chrome/browser/ssl/ios_chrome_security_state_model_client.mm (right):

https://codereview.chromium.org/2327433002/diff/180001/ios/chrome/browser/ssl...
ios/chrome/browser/ssl/ios_chrome_security_state_model_client.mm:10: #include
"ios/web/public/cert_store.h"
Is this include still necessary?

https://codereview.chromium.org/2327433002/diff/180001/ios/chrome/browser/ssl...
File ios/chrome/browser/ssl/ios_ssl_blocking_page.mm (right):

https://codereview.chromium.org/2327433002/diff/180001/ios/chrome/browser/ssl...
ios/chrome/browser/ssl/ios_ssl_blocking_page.mm:21: #include
"ios/web/public/cert_store.h"
ditto

https://codereview.chromium.org/2327433002/diff/180001/ios/web/net/crw_ssl_st...
File ios/web/net/crw_ssl_status_updater.mm (right):

https://codereview.chromium.org/2327433002/diff/180001/ios/web/net/crw_ssl_st...
ios/web/net/crw_ssl_status_updater.mm:10: #include "ios/web/public/cert_store.h"
ditto

https://codereview.chromium.org/2327433002/diff/180001/ios/web/net/crw_ssl_st...
File ios/web/net/crw_ssl_status_updater_unittest.mm (right):

https://codereview.chromium.org/2327433002/diff/180001/ios/web/net/crw_ssl_st...
ios/web/net/crw_ssl_status_updater_unittest.mm:12: #include
"ios/web/public/cert_store.h"
ditto

https://codereview.chromium.org/2327433002/diff/180001/ios/web/public/ssl_sta...
File ios/web/public/ssl_status.h (right):

https://codereview.chromium.org/2327433002/diff/180001/ios/web/public/ssl_sta...
ios/web/public/ssl_status.h:37: (certificate.get() ?
certificate->Equals(status.certificate.get())
Should this return false if |certificate| is null and |status.certificate| is
not null? Maybe move certificate comparison code to a separate function to make
the code simpler and easier to reason about?

[jam, 2016-09-08 16:59:25]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 16:59:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-09-08 17:03:24]

On 2016/09/08 16:57:46, Eugene But wrote:
> ios lgtm. Is there a reason why you use |if (!!certificate.get())| instead of
> |if (certificate)|? scoped_refptr implements |operator bool()|, so |if
> (certificate)| should work fine.

Will update.

Regarding removing cert_store files on ios, I was going to do that in later
patchsets (I did non-ios first). I'll update the cl.

Thanks

https://codereview.chromium.org/2327433002/diff/180001/ios/web/public/ssl_sta...
File ios/web/public/ssl_status.h (right):

https://codereview.chromium.org/2327433002/diff/180001/ios/web/public/ssl_sta...
ios/web/public/ssl_status.h:37: (certificate.get() ?
certificate->Equals(status.certificate.get())
On 2016/09/08 16:57:46, Eugene But wrote:
> Should this return false if |certificate| is null and |status.certificate| is
> not null? 

Yes that should return false since the certificates don't match. The code is
trying to do that by checking that either both don't have certificates, or both
do and they're equal.

> Maybe move certificate comparison code to a separate function to make
> the code simpler and easier to reason about?

[Ryan Sleevi, 2016-09-08 17:06:27]

Description was changed from

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with PlzNavigate
the process ID isn't known when the resource loading is happening.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with
PlzNavigate the process ID isn't known when the resource loading is
happening.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Ryan Sleevi, 2016-09-08 17:13:02]

jam@: Could you explain how this materially addresses the concerns raised when
this was proposed - in particular, with IPC overheads?

See
https://groups.google.com/a/chromium.org/d/msg/security-dev/OshNloC3r8o/v1Wb0...

From reviewing this CL, it doesn't seem like any of the concerns raised were
addressed, and so I'd be uncomfortable approving this.

[jam, 2016-09-08 17:27:43]

On 2016/09/08 17:13:02, Ryan Sleevi (slow) wrote:
> jam@: Could you explain how this materially addresses the concerns raised when
> this was proposed - in particular, with IPC overheads?

I don't see discussion related to IPC overhead in that thread (or maybe I'm
missing it, I did a scan).

Emily had mentioned there was concerns about IPC overhead earlier. First, note
that the IPC changes have already landed, so it's orthogonal with this patch.
The concern as I understand it was to send the certificates using the devtools
protocol to the devtools front end, which would be slow when debugging Android
devices. That was solved by only sending them to the devtools front end when the
"show certificate" button was pressed.

Certificates are sent to the renderer when devtools are open, but Pavel/Emily/I
didn't think that that would be a performance issue (i.e. they're dwarfed by
actual resources, and this is only when debugging).

I see you do also mention trusting the renderer more as a concern. Again, note
that this change doesn't affect that. The previous set of changes actually
reduced trust in the renderer. Previously, the security status (e.g. page lock
icon) was funnelled through the renderer to the browser, so a malicious renderer
could lie about that. That doesn't happen anymore (fixed in r415186). Similarly,
before my changes a malicious renderer could lie about which certificate was
used for the main page. Now after this specific change, that can't occur. That's
because devtools front end now just asks the browser to display a page's
certificate, whereas before it would give it the certificate id which is easily
changeable (since it's monotonically increasing and a renderer can do loads to
insert new entries).

> 
> See
>
https://groups.google.com/a/chromium.org/d/msg/security-dev/OshNloC3r8o/v1Wb0...
> 
> From reviewing this CL, it doesn't seem like any of the concerns raised were
> addressed, and so I'd be uncomfortable approving this.

[Ryan Sleevi, 2016-09-08 17:34:17]

On 2016/09/08 17:27:43, jam wrote:
> I don't see discussion related to IPC overhead in that thread (or maybe I'm
> missing it, I did a scan).

It's in the doc, along with a detailed plan about how to minimize this.

> Emily had mentioned there was concerns about IPC overhead earlier. First, note
> that the IPC changes have already landed, so it's orthogonal with this patch.
> The concern as I understand it was to send the certificates using the devtools
> protocol to the devtools front end, which would be slow when debugging Android
> devices. That was solved by only sending them to the devtools front end when
the
> "show certificate" button was pressed.

At the time the doc was written, certificates were attached to every response,
not just for DevTools, but because the need for the renderer to signal back to
the browser the need to show the security information. It's possible this has
been removed, and that the only remaining consumer was now DevTools. If this is
the case - that it's only DevTools - then that's the answer for how it was
meaningfully resolved, and it's a no brainer to stamp this :)

> That doesn't happen anymore (fixed in r415186).

Thanks, it sounds like that was the CL that was missing.

So long as we're now only serializing for DevTools, LGTM, and with great
appreciation to remove that :)

[jam, 2016-09-08 17:36:15]

On 2016/09/08 17:34:17, Ryan Sleevi (slow) wrote:
> On 2016/09/08 17:27:43, jam wrote:
> > I don't see discussion related to IPC overhead in that thread (or maybe I'm
> > missing it, I did a scan).
> 
> It's in the doc, along with a detailed plan about how to minimize this.

ah, I hadn't seen that doc before. These series of patches implement something
different from that doc.

> 
> > Emily had mentioned there was concerns about IPC overhead earlier. First,
note
> > that the IPC changes have already landed, so it's orthogonal with this
patch.
> > The concern as I understand it was to send the certificates using the
devtools
> > protocol to the devtools front end, which would be slow when debugging
Android
> > devices. That was solved by only sending them to the devtools front end when
> the
> > "show certificate" button was pressed.
> 
> At the time the doc was written, certificates were attached to every response,
> not just for DevTools, but because the need for the renderer to signal back to
> the browser the need to show the security information. It's possible this has
> been removed, and that the only remaining consumer was now DevTools. If this
is
> the case - that it's only DevTools - then that's the answer for how it was
> meaningfully resolved, and it's a no brainer to stamp this :)
> 
> > That doesn't happen anymore (fixed in r415186).
> 
> Thanks, it sounds like that was the CL that was missing.
> 
> So long as we're now only serializing for DevTools, LGTM, and with great
> appreciation to remove that :)

Thanks!

[jam, 2016-09-08 18:15:08]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 18:15:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-09-08 18:31:37]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 18:32:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 19:00:49]

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

[commit-bot: I haz the power, 2016-09-08 19:00:50]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jam, 2016-09-08 19:16:07]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 19:16:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-09-08 19:17:08]

Patchset #9 (id:230001) has been deleted

[pfeldman, 2016-09-08 20:37:52]

devtools lgtm

[commit-bot: I haz the power, 2016-09-08 20:57:43]

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

[commit-bot: I haz the power, 2016-09-08 20:57:44]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jam, 2016-09-08 23:47:55]

jam@chromium.org changed reviewers:
+ tsepez@chromium.org

[jam, 2016-09-08 23:47:56]

+tsepez for trivial removal of unused struct in common_param_traits_macros.h

[jam, 2016-09-08 23:48:42]

Description was changed from

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with
PlzNavigate the process ID isn't known when the resource loading is
happening.

BUG=598073, 504347
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with
PlzNavigate the process ID isn't known when the resource loading is
happening.

BUG=598073, 504347
TBR=tsepez
NOTRY=true
==========

[jam, 2016-09-08 23:48:50]

The CQ bit was checked by jam@chromium.org

[jam, 2016-09-08 23:48:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org, rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/2327433002/#ps250001
(title: "remove cert_store on ios")

[commit-bot: I haz the power, 2016-09-08 23:49:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-09-08 23:50:38]

felt mentioned offline no need to wait for her.

linux_site_isolation is red but it's independent of my change.
win_chromium_rel_ng also has a few failing layout tests that fail for all other
runs. both of these are not running without my patch because I have changes to
testing/buildbot. i could undo that part of the change and hit cq again but
given how slow the cq is with flakes and webkit tests, i don't want to wait more
so i'll just put on NOTRY=true to offset the failures above.

[commit-bot: I haz the power, 2016-09-08 23:55:39]

Description was changed from

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with
PlzNavigate the process ID isn't known when the resource loading is
happening.

BUG=598073, 504347
TBR=tsepez
NOTRY=true
==========

to

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with
PlzNavigate the process ID isn't known when the resource loading is
happening.

BUG=598073, 504347
TBR=tsepez
NOTRY=true
==========

[commit-bot: I haz the power, 2016-09-08 23:55:41]

Committed patchset #9 (id:250001)

[commit-bot: I haz the power, 2016-09-08 23:57:39]

Description was changed from

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with
PlzNavigate the process ID isn't known when the resource loading is
happening.

BUG=598073, 504347
TBR=tsepez
NOTRY=true
==========

to

==========
Stop using CertStore which is not compatible with PlzNavigate.

The reason is that certificate IDs depend on the process, and with
PlzNavigate the process ID isn't known when the resource loading is
happening.

BUG=598073, 504347
TBR=tsepez
NOTRY=true

Committed: https://crrev.com/8ae7cadae586d05e906fa581702e77c32080c4e7
Cr-Commit-Position: refs/heads/master@{#417444}
==========

[commit-bot: I haz the power, 2016-09-08 23:57:40]

Patchset 9 (id:??) landed as
https://crrev.com/8ae7cadae586d05e906fa581702e77c32080c4e7
Cr-Commit-Position: refs/heads/master@{#417444}