What's the 'right' way to gate code in the network service that normally would have been handled by //chrome/browser?

I ask because Expect-CT should never be a part of iOS, Android, nor is it intended for non-Google-Chrome builds - that is, this functionally is to support a Google-specific feature. While I agree that resolving these tests (or whatever is failing beside them) is a priority, it seems an unfortunate side-effect of refactoring that brought it into the network service if this code is now being included in iOS, where it will never be used, but where I'd also be surprised if the code can be stripped out effectively.

I'm not really sure - mojom files, unfortunately, doesn't support #ifdefs.  I'd exclude the relevant cc files and use #ifdefs on the code to call them, and the same for tests, excluding / ifdefing as appropriate, much like you would with code in net, the only different is the mojom+traits files may require linking in structs not used on some platforms.

Don't they? I wasn't sure if https://chromium.googlesource.com/chromium/src/+/master/docs/security/mojo.md#do-use-enableif-to-guard-platform_specific-constructs was for that purpose.

"enable_expect_ct_reporting" in NetworkContextParams ( https://chromium.googlesource.com/chromium/src/+/b52e5772bea1dd76f60fea8d1275d028b253a88a/services/network/public/mojom/network_context.mojom#171 ) is only relevant iff ct_logs is non-empty and "enforce_chrome_ct_policy" ( https://chromium.googlesource.com/chromium/src/+/b52e5772bea1dd76f60fea8d1275d028b253a88a/services/network/public/mojom/network_context.mojom#116 ) is set. Otherwise, CT compliance will never be checked, and Expect-CT will never run.

Expect-CT is also dependent on kDynamicExpectCTFeature ( https://cs.chromium.org/chromium/src/net/http/transport_security_state.h?rcl=eebbaf9cc76e0993b1edb05473a8989a7a99a6d4&l=341 ) and/or the static list ( https://cs.chromium.org/chromium/src/net/http/transport_security_state.cc?rcl=eebbaf9cc76e0993b1edb05473a8989a7a99a6d4&l=411 ). The former is enabled for all platforms (but not relevant to iOS), the latter is explicitly disabled for Android and iOS.


I mention all of this, because while a short-term solution is likely "just disable the test if it's causing issues", that should probably only be done if we ensure the code-being-tested can't be reached for iOS/Android. There's likely an expedient solution separate from a maintainable solution, so I'm mentioning the "intent" in the hopes we can find a solution that meets both.

Maybe they do - if they do, it's the first I've heard of it.  Looks like the documentation on it in that file, at least, was only added 7 months ago.  :(

I'll look into disabling the API on Android and iOS - agree that we shouldn't just disable the tests.  May not get to it this week, unless we start getting more reports of this failing.

Unassigning from myself.  A lot more stuff here than I thought I'd signed up for - I'm not a big fan of hacking out the net_internals_ui dependencies here on Android with ifdefs (As it uses the ExpectCTReporter), or bifurcating net-internals domain_security_policy_view.js to hide stuff that doesn't work under Android.  Not a huge effort, but more of a timesink than I care to spend on it.

Actually, I can still disable it on iOS, and just leave it on in Android.  (Yes, I'm fickle).  I'll wait for net-internals to be switched over to the NetworkService, since that has some CT-related API additions.

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

commit 74721be1acc7edb33f4a9682f8ab527ad1a323cc
Author: Matt Menke <mmenke@chromium.org>
Date: Wed Oct 31 22:08:10 2018

NetworkService: Disable Certificate Transparency on iOS.

We don't support CT on iOS, so this CL removes the relevant
NetworkService APIs and objects on iOS. We also don't support
it on Android, but removing the APIs there would require a
fair bit of work on the net-internals code.

Bug:  887122 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I012ea593f1df5a25a9379f57a268225ba2d42381
Reviewed-on: https://chromium-review.googlesource.com/c/1249884
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604402}
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/BUILD.gn
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/network_context.cc
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/network_context.h
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/network_context_unittest.cc
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/network_service.cc
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/network_service.h
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/public/cpp/BUILD.gn
[add] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/public/cpp/features.gni
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/public/cpp/typemaps.gni
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/public/mojom/BUILD.gn
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/public/mojom/network_service.mojom
[modify] https://crrev.com/74721be1acc7edb33f4a9682f8ab527ad1a323cc/services/network/test/test_network_context.h