Skip to content

Commit

Permalink
Revert "Revert "GWP-ASan: Add feature map""
Browse files Browse the repository at this point in the history
This reverts commit 6080f92.

Reason for revert:
LUCI Bisection identified this CL as the culprit of a build failure. See the analysis: https://luci-bisection.appspot.com/analysis/b/8769956037531365153

Sample failed build: https://ci.chromium.org/b/8769956037531365153

If this is a false positive, please report it at https://bugs.chromium.org/p/chromium/issues/entry?comment=Analysis%3A+https%3A%2F%2Fluci-bisection.appspot.com%2Fanalysis%2Fb%2F8769956037531365153&components=Tools%3ETest%3EFindit&labels=LUCI-Bisection-Wrong%2CPri-3%2CType-Bug&status=Available&summary=Wrongly+blamed+https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fsrc%2F%2B%2F4866672

Original change's description:
> Revert "GWP-ASan: Add feature map"
>
> This reverts commit 0f1f9dc.
>
> Reason for revert: This change is causing compilation errors in android-builder-perf builds (https://ci.chromium.org/ui/p/chrome/builders/ci/android-builder-perf?limit=100&cursor=id%3E8769985043865326721)
>
> Original change's description:
> > GWP-ASan: Add feature map
> >
> > We intend to expand the GWP-ASan Android experiment to include WebView.
> > WebView experiments must add a flag to
> > `ProductionSupportedFlagList.java`. To do so, we need to expose the two
> > GWP-ASan `base::Feature`s to Java. This CL plumbs together the
> > boilerplate to do so.
> >
> > This change was generated by following
> > `android_accessing_cpp_features_in_java.md`.
> >
> > Bug: b/287426302
> > Low-Coverage-Reason: Trivial JNI glue
> > Change-Id: Ia5deb8c36c2d52fcdc2acd5ea2837c42bfaaa6ad
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4843470
> > Commit-Queue: Kalvin Lee <kdlee@chromium.org>
> > Reviewed-by: Benjamin Joyce (Ben) <bjoyce@chromium.org>
> > Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
> > Reviewed-by: Bo Liu <boliu@chromium.org>
> > Reviewed-by: Matthew Denton <mpdenton@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1196407}
>
> Bug: b/287426302 chromium:1482658
> Change-Id: If203c21de0ef17f147b73f33c3dfb7ef2199839e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4866672
> Reviewed-by: Matthew Denton <mpdenton@chromium.org>
> Reviewed-by: Bo Liu <boliu@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Reviewed-by: John Chen <johnchen@chromium.org>
> Commit-Queue: Ashwin Verleker <ashwinpv@google.com>
> Reviewed-by: Gregory Guterman <guterman@google.com>
> Reviewed-by: Benjamin Joyce (Ben) <bjoyce@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1196711}
>

Bug: b/287426302 chromium:1482658
Change-Id: I68bc2b5eec617cb6cbf15e9972a4f6724324b3be
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4865095
Commit-Queue: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Owners-Override: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Bot-Commit: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1196740}
  • Loading branch information
luci-bisection@appspot.gserviceaccount.com authored and Chromium LUCI CQ committed Sep 14, 2023
1 parent 8ee8e3f commit a627f86
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 0 deletions.
2 changes: 2 additions & 0 deletions android_webview/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,7 @@ android_library("common_java") {
"//components/autofill/android:autofill_features_java",
"//components/embedder_support/android/metrics:java",
"//components/feature_engagement/public:public_java",
"//components/gwp_asan/client/android:gwp_asan_java",
"//components/metrics:metrics_java",
"//components/network_session_configurator/android:network_session_configurator_java",
"//components/permissions/android:core_java",
Expand All @@ -811,6 +812,7 @@ android_library("common_java") {
"//ui/accessibility:accessibility_features_java",
"//ui/android:ui_android_features_java",
]

srcjar_deps = [
":common_java_features_srcjar",
":common_java_switches_srcjar",
Expand Down
1 change: 1 addition & 0 deletions chrome/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,7 @@ if (current_toolchain == default_toolchain) {
"//chrome/browser/ui/android/webid/internal:java",
"//components/autofill/android:payments_autofill_java",
"//components/browser_ui/bottomsheet/android/internal:java",
"//components/gwp_asan/client/android:gwp_asan_java",
"//components/messages/android/internal:java",
"//components/segmentation_platform/internal:internal_java",
]
Expand Down
5 changes: 5 additions & 0 deletions components/gwp_asan/client/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ source_set("features") {
"//base",
]
configs += [ ":gwp_asan_implementation" ]

if (is_android) {
sources += [ "feature_map.cc" ]
deps += [ "android:gwp_asan_headers" ]
}
}

component("client") {
Expand Down
30 changes: 30 additions & 0 deletions components/gwp_asan/client/android/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//build/config/android/rules.gni")

generate_jni("gwp_asan_headers") {
sources = [ "org/chromium/components/gwp_asan/GwpAsanFeatureMap.java" ]
}

java_cpp_features("java_features_srcjar") {
# External code should depend on `:gwp_asan_java` instead.
visibility = [ ":*" ]
sources = [ "//components/gwp_asan/client/gwp_asan_features.cc" ]
template = "org/chromium/components/gwp_asan/GwpAsanFeatures.java.tmpl"
}

android_library("gwp_asan_java") {
srcjar_deps = [
":gwp_asan_headers",
":java_features_srcjar",
]
deps = [
"//base:base_java",
"//base:jni_java",
"//build/android:build_java",
"//components/gwp_asan/client:features",
]
sources = [ "org/chromium/components/gwp_asan/GwpAsanFeatureMap.java" ]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.components.gwp_asan;

import org.chromium.base.FeatureMap;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;

/**
* Java accessor for base::Features listed in components/gwp_asan/client/feature_map.cc.
* Patterned after example linked from `android_accessing_cpp_features_in_java.md`.
*/
@JNINamespace("gwp_asan::android")
public class GwpAsanFeatureMap extends FeatureMap {
private static final GwpAsanFeatureMap sInstance = new GwpAsanFeatureMap();

// Do not instantiate this class.
private GwpAsanFeatureMap() {}

/**
* @return the singleton {@link GwpAsanFeatureMap}
*/
public static GwpAsanFeatureMap getInstance() {
return sInstance;
}

/**
* Convenience method to call {@link #isEnabledInNative(String)} statically.
*/
public static boolean isEnabled(String featureName) {
return getInstance().isEnabledInNative(featureName);
}

@Override
protected long getNativeMap() {
return GwpAsanFeatureMapJni.get().getNativeMap();
}

@NativeMethods
public interface Natives {
long getNativeMap();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.components.gwp_asan;

/**
* Contains features that are specific to GWP-ASan.
*/
public final class GwpAsanFeatures {{

{NATIVE_FEATURES}

// Prevents instantiation.
private GwpAsanFeatures() {{}}
}}
38 changes: 38 additions & 0 deletions components/gwp_asan/client/feature_map.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/android/feature_map.h"

#include <jni.h>

#include "base/feature_list.h"
#include "base/no_destructor.h"
#include "components/gwp_asan/client/android/gwp_asan_headers/GwpAsanFeatureMap_jni.h"
#include "components/gwp_asan/client/gwp_asan_features.h"

// This source-only file is boilerplate generated by following
// `android_accessing_cpp_features_in_java.md`.

namespace gwp_asan::android {
namespace {

const base::Feature* const kFeaturesExposedToJava[] = {
&internal::kGwpAsanMalloc,
&internal::kGwpAsanPartitionAlloc,
};

// static
base::android::FeatureMap* GetFeatureMap() {
static base::NoDestructor<base::android::FeatureMap> kFeatureMap(std::vector(
std::begin(kFeaturesExposedToJava), std::end(kFeaturesExposedToJava)));
return kFeatureMap.get();
}

} // namespace

static jlong JNI_GwpAsanFeatureMap_GetNativeMap(JNIEnv* env) {
return reinterpret_cast<jlong>(GetFeatureMap());
}

} // namespace gwp_asan::android
8 changes: 8 additions & 0 deletions components/memory_system/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ source_set("memory_system") {
}
}

# Regardless of `enable_gwp_asan`, we must depend directly
# on the presence of the `base::Feature` that governs it. This is
# because WebView depends on it, but support cannot be "compiled out"
# hinging on a buildflag.
if (is_android) {
deps += [ "//components/gwp_asan/client:features" ]
}

if (is_chromeos) {
deps += [ "//build:chromeos_buildflags" ]
}
Expand Down

0 comments on commit a627f86

Please sign in to comment.