rsleevi@ has fixed this issue as it broke Chrome as well.

It occurred to me that excluding real CT logs from Cronet could reduce its binary size. 

Indeed building https://codereview.chromium.org/2493863002/ on iOS saves 50k of uncompressed binary size for single arm64 architecture:

Normal:
-rwxr-xr-x   1 mef  eng  5566288 Nov 15 17:38 Cronet

With CT Ignores:
-rwxr-xr-x   1 mef  eng  5518112 Nov 15 17:46 Cronet

Savings for fat binary on iOS should be approximately double, but on Android it maybe a little trickier as UrlRequestContextBuilder references the default CT code, which will likely pull in logs data.

That sounds like there may be confounding factors - or at least worth digging in where that size comes from. As you can see from the list of known logs, the list is only a few k at best. So 50k is unreasonably significant and worth digging in to validate.

Also, this is a security feature we want and need on all platforms, and for all embedders. Measuring size here in order to remove would be akin to suggesting we should distrust CAs to save bytes. However, measuring size to make sure we aren't doing something terribly inefficient is totally useful and helpful to optimize when appropriate :)

Great points, I'll follow up on that.

It turns out that difference between properly stripped release binaries is only 50k for the fat binaries, and is mostly due to the code. 

I've built Cronet.framework with and without my CL and ran the following command to get symbols with sizes but without addresses:

symbols -noSources Cronet | awk '{$1=""; print $0}'

and than ran the diff (attached).

I'd say this is working as intended and is a reasonable price to pay for increased security.

Deleted: sizes_diff.txt 48.1 KB

sizes_diff.txt 48.1 KB View Download

Deleted: sizes_with_ct.txt 4.0 MB

sizes_with_ct.txt 4.0 MB View Download

Deleted: sizes_no_ct.txt 4.0 MB

sizes_no_ct.txt 4.0 MB View Download

That actually sounds like a really large cost for code - 40K for CT support? I CC'd eran to see if we can't optimize some of that stuff in the background/parallel and figure out if there's low hanging fruit. Thanks for double checking and confirming though.

Eran: We should figure out what Cronet's intended level of CT support should be. It won't have a component updater, so the entire SCT inclusion proof fetching mechanism may be superfluous for cronet code. If so, we can probably optimize the code so that the linker has a better chance at dead code elimination. We'd still pay the cost for the logs and SCT validation, but consistency proof checking in the absence of STH delivery may be something we delay until post-Chrome implementation.

I don't want to hijack this bug too much, so let's file new bugs with concrete actions after we have a chance to investigate mef's result.

Do I understand correctly that the size saving comes from the linker not linking in the "real" CTVerifier (MultiLogCTVerifier) ?

Since all inclusion checking code lives under //components/certificate_transparency one option would be to exclude this dependency entirely from cronet's (and other embedders) builds.

To facilitate linker optimizations we should either not create a TreeStateTracker in the same contexts that the CTVerifier is created or abstract the TreeStateTracker interface so a fake one can be created.

Does that make sense? Is that what you're after?

Yes, I think that size savings are coming from not linking in the real CTVerifier. 

Take a look at sizes_diff.txt from Comment 6 which shows symbols which are different between no-op and real implementation (IIUIC the first column is size ).

Bear in mind that fat binary is armv7+arm64, so the per-architecture overhead is just around 25k.

Per conversation with rsleevi@ Cronet could benefit from explicitly using a 'no-op' CT verifier to save some CPU and processing time.

The CL from Comment 2 would do approximately that, but needs to be updated to current state. 

Unfortunately naively supplying no-op CTVerifier to UrlRequestContextBuilder would NOT reduce the binary size as MultiLogCTVerifier will still be referenced by runtime check. 

We should consider doing some compile-time configuration setting to remove unused CT code from Cronet.

There are internal reports (b/62956264) of crashes in net::CTLogVerifier::CTLogVerifier() in Cronet on iOS.

This makes switching to no-op CT Verifier a higher priority.

I would suggest that there is an underlying issue you should investigate, because it's almost entirely unrelated to this code :)

Confirmed, this is not caused by CTLogVerifier code and is remedied by other changes.

Lowering priority, but created https://chromium-review.googlesource.com/c/565647/ to address this as Cronet net log shows few milliseconds spent on CT verification.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d036c59734ff063287a2a896fb9474567fe0098

commit 6d036c59734ff063287a2a896fb9474567fe0098
Author: Misha Efimov <mef@chromium.org>
Date: Tue Jul 11 18:51:34 2017

[Cronet] Use DoNothingCTLogVerifier and DoNothingCTPolicyEnforcer.

Bug:  663883 
Change-Id: Ibf81b00cfa1d1999bf544ea197886576fbb62a20
Reviewed-on: https://chromium-review.googlesource.com/565647
Commit-Queue: Misha Efimov <mef@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485685}
[modify] https://crrev.com/6d036c59734ff063287a2a896fb9474567fe0098/components/cronet/url_request_context_config.cc
[modify] https://crrev.com/6d036c59734ff063287a2a896fb9474567fe0098/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/6d036c59734ff063287a2a896fb9474567fe0098/net/url_request/url_request_context_builder.h