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

commit 875a2fa43caab0ee82efe68fb5adb357392434ad
Author: gab <gab@chromium.org>
Date: Tue May 16 19:47:52 2017

Enable DCHECKs by default in non-official builds.

BUG= 722827 
TBR=sdefresne@chromium.org for ios/

Review-Url: https://codereview.chromium.org/2886803002
Cr-Commit-Position: refs/heads/master@{#472195}

[modify] https://crrev.com/875a2fa43caab0ee82efe68fb5adb357392434ad/build/config/dcheck_always_on.gni
[modify] https://crrev.com/875a2fa43caab0ee82efe68fb5adb357392434ad/ios/web/web_state/ui/wk_web_view_configuration_provider.mm

Woohoo!

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

commit 25ae5928489058ac5bf0d0e7230f8dc5873da175
Author: dmazzoni <dmazzoni@chromium.org>
Date: Tue May 16 22:14:31 2017

Fix compile error in audio_manager_cras when dcheck_always_on=true

This is speculative as I can't easily compile with use_cras=true
without updating a full chrome os checkout first

BUG= 722827 
TBR=justinlin
NOTRY=true

Review-Url: https://codereview.chromium.org/2886993002
Cr-Commit-Position: refs/heads/master@{#472227}

[modify] https://crrev.com/25ae5928489058ac5bf0d0e7230f8dc5873da175/media/audio/cras/audio_manager_cras.cc

Possibly causing at least two failures, maybe more

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

commit 8cc767cb0d1c65af6d5a36858c9633c929b7e4cd
Author: findit-for-me <findit-for-me@appspot.gserviceaccount.com>
Date: Tue May 16 23:14:54 2017

Revert of Enable DCHECKs by default in non-official builds. (patchset #3 id:40001 of https://codereview.chromium.org/2886803002/ )

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 472195 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzg3NWEyZmE0M2NhYWIwZWU4MmVmZTY4ZmI1YWRiMzU3MzkyNDM0YWQM

Original issue's description:
> Enable DCHECKs by default in non-official builds.
>
> BUG= 722827 
> TBR=sdefresne@chromium.org for ios/
>
> Review-Url: https://codereview.chromium.org/2886803002
> Cr-Commit-Position: refs/heads/master@{#472195}
> Committed: https://chromium.googlesource.com/chromium/src/+/875a2fa43caab0ee82efe68fb5adb357392434ad

TBR=brettw@chromium.org,thakis@chromium.org,sdefresne@chromium.org,gab@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 722827 , 723090 , 723049 

Review-Url: https://codereview.chromium.org/2892473002
Cr-Commit-Position: refs/heads/master@{#472242}

[modify] https://crrev.com/8cc767cb0d1c65af6d5a36858c9633c929b7e4cd/build/config/dcheck_always_on.gni
[modify] https://crrev.com/8cc767cb0d1c65af6d5a36858c9633c929b7e4cd/ios/web/web_state/ui/wk_web_view_configuration_provider.mm

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

commit ab611961b4bf7919c37a26491cc79376a4b07f7b
Author: gab <gab@chromium.org>
Date: Wed May 17 14:01:05 2017

Fix DCHECK_IS_ON() iOS build

Extracted from https://codereview.chromium.org/2886803002/

BUG= 722827 
TBR=sdefresne

Review-Url: https://codereview.chromium.org/2885403002
Cr-Commit-Position: refs/heads/master@{#472445}

[modify] https://crrev.com/ab611961b4bf7919c37a26491cc79376a4b07f7b/ios/web/web_state/ui/wk_web_view_configuration_provider.mm

Please don't reenable this without checking https://bugs.chromium.org/p/chromium/issues/detail?id=723758. We can't have another massive ClusterFuzz breakage to manage.

I don't think we reached a consensus that we should do this in that thread (in fact, I would've said that the consensus was that we shouldn't do this). I'm WontFixing this, though we can always go another round on this if you want :).

To be clear: I think we have two somewhat-conflicting goals:

1) We should make sure that DCHECKs aren't firing and those that are get fixed or disabled.

2) We should test the configuration we ship as much as possible.

If we were to make this change, we might get more people fixing dchecks, but it's not entirely clear that that's true. However, we would certainly get less coverage unless we *also* made sure that the bots all turned this off in a release config, which the linked CL didn't do.

Separate from that, as brettw@ raised on the thread, there's the confusion over what you'd expect a "release" build to actually mean. In my world, I wouldn't expect a release build to contain debug-only assertions, hence it just seems inherently confusing (counterintuitive) to have them on. It's unclear if there's a particularly good answer here for this.

Re. (1) and whether it's worth it: many true DCHECK errors were filed and fixed from the few hours this was enabled so it is definitely worth it IMO (ref. fixed blockers on this bug and more).

Re. (2) we already have dcheck_always_on=true on most bots AFAIK. This change was merely meant to affect developers not bots. I'd like to propose we bind the default to is_component_build instead of !is_official_build. I think this would hit the majority of developers whom aren't running a build with strict performance requirements while not affecting bots intended to test production builds (configurations highlighted as hitting problems with the first attempt would be fine with the latter proposition).

Looks like all blockers were fixed and Albatros caught the trickier ones which I assume are fixed by now. We do have a steady stream (last I heard from Albatros team) though but our rule to revert any CL that causes new DCHECKs should address this.

Question : are we happy with relying on Albatros or do we want to bring the error to the developers when possible? (still need Albatros as sometimes the DCHECK is only in some configs untouched by developers and bots)

I'd be inclined to rely solely on Albatros (I initially tried to do this before we had Albatros and desperately needed stronger coverage).

I'm usually an advocate of sooner-is-better to catch programming errors but since developers can opt-in to dcheck_always_on we effectively already have that. 

Question is therefore should it be forced by default on all devs?

@wez : thoughts?

My 2c: Only enable DCHECKs, by default, in Debug builds, otherwise there is more likely to be breakage landed in Official builds (without DCHECKs), which would previously have been covered by (un-Official) Release.

Albatross should be sufficient for us to burn down DCHECK debt, and catch new regressions, once we bring it back up.

Okay SGTM, while I do think most devs should run with dcheck_always_on (when not running Debug): I agree that Albatros, now that we have it, is the best way to burn down existing ones and catch regressions. Devs that get bitten can always retroactively enable dcheck_always_on.