Occasional DCHECK - CheckFeatureIdentity in feature_list.cc |
|||||
Issue descriptionMostly just harmless noise when debugging component builds with dcheck on. Its easy to comment out the dcheck with no apparent ill effects. It looks like the issue is that CheckFeatureIdentity tries to verify that there is only a single instance of each feature. However, on component builds where multiple components link in chrome/common:constants, and multiple components check the same feature, the check fails. In particular, we are hitting on when entering webxr presentation on android about IncognitoStrings.
,
Aug 9
It only fails on component debug builds. The feature is first registered here: base::FeatureList::CheckFeatureIdentity base::FeatureList::IsFeatureEnabled chrome::android::JNI_ChromeFeatureList_IsEnabled Java_org_chromium_chrome_browser_ChromeFeatureList_nativeIsEnabled picking up the kIncognitoStrings base::Feature from /lib/arm/libchrome.cr.so. Later we check the feature from vr::UiSceneCreator::CreateOverflowMenu. Picking up the kIncognitoStrings base::Feature from /lib/arm/libvr_common.cr.so checks later. We could have the features in a component, and exported so everyone gets the same values, but its probably inefficient. Adding FeatureList.cc owners for recommendations.
,
Aug 9
After thinking about this more - I see some features like device/base/features.h are exported from components. It looks like maybe the bug is that all features should be used in the component they are defined in, or exported from a component, but never defined in a static library.
,
Aug 9
Right - I think exporting the features is the correct fix for those hitting this.
,
Aug 10
,
Aug 14
We just stumbled on this again. I'll pick up the bug.
,
Aug 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d33615ea8ee82c31b1aafaa78691d43a6db4152c commit d33615ea8ee82c31b1aafaa78691d43a6db4152c Author: Christopher Grant <cjgrant@chromium.org> Date: Tue Aug 21 18:19:43 2018 Move chrome features to a component By moving the feature source into a component, we won't end up with multiple instances of the feature constants across multiple component build libraries. This should eliminate DCHECK failures that crop up when components check if features are enabled. BUG= 872073 Change-Id: Ia82e990596d52076027499fe23b73c870c64b5e1 Reviewed-on: https://chromium-review.googlesource.com/1177895 Commit-Queue: Christopher Grant <cjgrant@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#584834} [modify] https://crrev.com/d33615ea8ee82c31b1aafaa78691d43a6db4152c/chrome/common/BUILD.gn [modify] https://crrev.com/d33615ea8ee82c31b1aafaa78691d43a6db4152c/chrome/common/chrome_features.h
,
Sep 4
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by mthiesse@chromium.org
, Aug 8