Skip to content

Commit

Permalink
Automatically annotate dangling ExperimentalAsh pointers [6/N]
Browse files Browse the repository at this point in the history
This patch is:
- Automated. See [automation] section.
- Not affecting users, only Chrome's CQ. See [no-op] section.

This is a split affecting:
- components/

Context:
--------
@bartekn is going to remove the `ExperimentalAsh` raw_ptr annotation.
This was a performance experiment to enable/disable MiraclePtr on a
subset of new pointers added for ChromeOS ash (non-lacros).

Before removing it, he needs to enable it by default in code. Enabling
MiraclePtr for them, causes the DanglingPointerDetector to be enforced.
To pass the CQ, we need to annotate pre-existing dangling pointer with
`DanglingUntriaged`. After enabling ExperimentalAsh feature, it won't be
allowed anymore for those pointers to become dangling.

Automation:
-----------
1. Enable the `PartitionAllocBackupRefPtrForAsh` feature flag.
2. Apply the 'annotator' CL: https://chromium-review.googlesource.com/c/chromium/src/+/4474553
3. Run all the gtests. See https://docs.google.com/document/d/1AMMERcqy0eafFWopUCHYsIKIKEp3J8DFxqW9UIbzIHo
4. Concatenate all the output_*, `filter`, `sort -nr`, `uniq`.
5. Run https://github.com/ArthurSonzogni/chrome-dangling-ptr-apply-edit
6. Run `git cl format`
7. Cherry-pick the patch created from (5)

no-op:
------
The DanglingPointerDetector is not enabled against users. It only
affects tests and chromium developers.

Cq-Include-Trybots: luci.chrome.try:chromeos-betty-pi-arc-chrome;luci.chrome.try:linux-chromeos-chrome
Bug: chromium:1441101
Change-Id: I5fbafc0361bed63dc114d03d1df4369fff21b1f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4774464
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184946}
  • Loading branch information
ArthurSonzogni authored and Chromium LUCI CQ committed Aug 17, 2023
1 parent 4a9eff0 commit a4ef6ce
Show file tree
Hide file tree
Showing 31 changed files with 65 additions and 44 deletions.
3 changes: 2 additions & 1 deletion components/exo/buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ class Buffer::Texture : public viz::ContextLostObserver {
void ScheduleWaitForRelease(base::TimeDelta delay);
void WaitForRelease();

const raw_ptr<gfx::GpuMemoryBuffer, ExperimentalAsh> gpu_memory_buffer_;
const raw_ptr<gfx::GpuMemoryBuffer, DanglingUntriaged | ExperimentalAsh>
gpu_memory_buffer_;
const gfx::Size size_;
scoped_refptr<viz::RasterContextProvider> context_provider_;
const unsigned texture_target_;
Expand Down
4 changes: 3 additions & 1 deletion components/exo/client_controlled_shell_surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,9 @@ class ClientControlledWindowStateDelegate : public ash::WindowStateDelegate {

private:
raw_ptr<ClientControlledShellSurface, ExperimentalAsh> shell_surface_;
raw_ptr<ash::ClientControlledState::Delegate, ExperimentalAsh> delegate_;
raw_ptr<ash::ClientControlledState::Delegate,
DanglingUntriaged | ExperimentalAsh>
delegate_;
};

bool IsPinned(const ash::WindowState* window_state) {
Expand Down
5 changes: 3 additions & 2 deletions components/exo/data_device_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ class TestDataDeviceDelegate : public DataDeviceDelegate {
private:
std::vector<DataEvent> events_;
std::unique_ptr<DataOffer> data_offer_;
raw_ptr<Surface, ExperimentalAsh> entered_surface_ = nullptr;
raw_ptr<Surface, DanglingUntriaged | ExperimentalAsh> entered_surface_ =
nullptr;
bool can_accept_data_events_for_surface_ = true;
};

Expand All @@ -135,7 +136,7 @@ class TestSeat : public Seat {
Surface* GetFocusedSurface() override { return surface_; }

private:
raw_ptr<Surface, ExperimentalAsh> surface_ = nullptr;
raw_ptr<Surface, DanglingUntriaged | ExperimentalAsh> surface_ = nullptr;
};

class DataDeviceTest : public test::ExoTestBase {
Expand Down
3 changes: 2 additions & 1 deletion components/exo/data_offer.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ class DataOffer final : public ui::PropertyHandler {
void OnPickledUrlsResolved(SendDataCallback callback,
const std::vector<GURL>& urls);

const raw_ptr<DataOfferDelegate, ExperimentalAsh> delegate_;
const raw_ptr<DataOfferDelegate, DanglingUntriaged | ExperimentalAsh>
delegate_;

// Data for a given mime type may not ever be requested, or may be requested
// more than once. Using callbacks and a cache allows us to delay any
Expand Down
2 changes: 1 addition & 1 deletion components/exo/display_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class DisplayTest : public test::ExoTestBase {
}

private:
raw_ptr<TestPropertyResolver, ExperimentalAsh> resolver_;
raw_ptr<TestPropertyResolver, DanglingUntriaged | ExperimentalAsh> resolver_;
};

TEST_F(DisplayTest, CreateSurface) {
Expand Down
4 changes: 2 additions & 2 deletions components/exo/drag_drop_operation_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ class DragDropOperationTestWithWebUITabStripTest
MockShellDelegate* mock_shell_delegate() { return mock_shell_delegate_; }

private:
raw_ptr<NiceMock<MockShellDelegate>, ExperimentalAsh> mock_shell_delegate_ =
nullptr;
raw_ptr<NiceMock<MockShellDelegate>, DanglingUntriaged | ExperimentalAsh>
mock_shell_delegate_ = nullptr;
base::test::ScopedFeatureList scoped_feature_list_;
};

Expand Down
2 changes: 1 addition & 1 deletion components/exo/extended_drag_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class ExtendedDragSource : public DataSourceObserver,

// Created and destroyed at wayland/zcr_extended_drag.cc and its lifetime is
// tied to the zcr_extended_drag_source_v1 object it's attached to.
const raw_ptr<Delegate, ExperimentalAsh> delegate_;
const raw_ptr<Delegate, DanglingUntriaged | ExperimentalAsh> delegate_;

// The pointer location in screen coordinates.
gfx::PointF pointer_location_;
Expand Down
4 changes: 2 additions & 2 deletions components/exo/extended_drag_source_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ class ExtendedDragSourceTest : public test::ExoTestBase {
exo_test_helper()->CreateGpuMemoryBuffer(size));
}

raw_ptr<ash::DragDropController, ExperimentalAsh> drag_drop_controller_ =
nullptr;
raw_ptr<ash::DragDropController, DanglingUntriaged | ExperimentalAsh>
drag_drop_controller_ = nullptr;
std::unique_ptr<Seat> seat_;
std::unique_ptr<DataSource> data_source_;
std::unique_ptr<ExtendedDragSource> extended_drag_source_;
Expand Down
3 changes: 2 additions & 1 deletion components/exo/gaming_seat.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ class GamingSeat : public aura::client::FocusChangeObserver,

private:
// The delegate that handles gamepad_added.
const raw_ptr<GamingSeatDelegate, ExperimentalAsh> delegate_;
const raw_ptr<GamingSeatDelegate, DanglingUntriaged | ExperimentalAsh>
delegate_;

// Contains the delegate for each gamepad device.
base::flat_map<int, std::unique_ptr<Gamepad>> gamepads_;
Expand Down
2 changes: 1 addition & 1 deletion components/exo/pointer.h
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ class Pointer : public SurfaceTreeHost,
const gfx::PointF& location_in_target);

// The delegate instance that all events are dispatched to.
const raw_ptr<PointerDelegate, ExperimentalAsh> delegate_;
const raw_ptr<PointerDelegate, DanglingUntriaged | ExperimentalAsh> delegate_;

const raw_ptr<Seat, ExperimentalAsh> seat_;

Expand Down
5 changes: 3 additions & 2 deletions components/exo/pointer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,9 @@ class PointerConstraintTest : public PointerTest {
testing::NiceMock<MockPointerConstraintDelegate> constraint_delegate_;
testing::NiceMock<MockPointerDelegate> delegate_;
std::unique_ptr<ShellSurface> shell_surface_;
raw_ptr<Surface, ExperimentalAsh> surface_;
raw_ptr<aura::client::FocusClient, ExperimentalAsh> focus_client_;
raw_ptr<Surface, DanglingUntriaged | ExperimentalAsh> surface_;
raw_ptr<aura::client::FocusClient, DanglingUntriaged | ExperimentalAsh>
focus_client_;
};

// Instantiate the values of disabling/enabling reactive frame submission in the
Expand Down
2 changes: 1 addition & 1 deletion components/exo/shell_surface_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ class CustomWindowTargeter : public aura::WindowTargeter {
}

raw_ptr<ShellSurfaceBase, ExperimentalAsh> shell_surface_;
const raw_ptr<views::Widget, ExperimentalAsh> widget_;
const raw_ptr<views::Widget, DanglingUntriaged | ExperimentalAsh> widget_;
};

void CloseAllShellSurfaceTransientChildren(aura::Window* window) {
Expand Down
3 changes: 2 additions & 1 deletion components/exo/shell_surface_presentation_time_recorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ class ShellSurfacePresentationTimeRecorder
const gfx::PresentationFeedback& feedback);

private:
raw_ptr<ShellSurface, ExperimentalAsh> shell_surface_ = nullptr;
raw_ptr<ShellSurface, DanglingUntriaged | ExperimentalAsh> shell_surface_ =
nullptr;
std::unique_ptr<Reporter> reporter_;

uint64_t next_request_id_ = 0u;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ class ShellSurfacePresentationTimeRecorderTest : public test::ExoTestBase {
protected:
std::unique_ptr<ShellSurface> shell_surface_;
std::unique_ptr<TestRecorder> recorder_;
raw_ptr<TestReporter, ExperimentalAsh> reporter_ = nullptr;
raw_ptr<TestReporter, DanglingUntriaged | ExperimentalAsh> reporter_ =
nullptr;
};

TEST_F(ShellSurfacePresentationTimeRecorderTest, Request) {
Expand Down
3 changes: 2 additions & 1 deletion components/exo/test/shell_surface_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ namespace {
// Internal structure that owns buffer, surface and subsurface instances.
// This is owned by the host window as an owned property.
struct Holder {
raw_ptr<exo::Surface, ExperimentalAsh> root_surface = nullptr;
raw_ptr<exo::Surface, DanglingUntriaged | ExperimentalAsh> root_surface =
nullptr;
std::vector<std::tuple<std::unique_ptr<exo::Buffer>,
std::unique_ptr<exo::Surface>,
std::unique_ptr<exo::SubSurface>>>
Expand Down
2 changes: 1 addition & 1 deletion components/exo/touch.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class Touch : public ui::EventHandler,
void CancelAllTouches();

// The delegate instance that all events are dispatched to.
const raw_ptr<TouchDelegate, ExperimentalAsh> delegate_;
const raw_ptr<TouchDelegate, DanglingUntriaged | ExperimentalAsh> delegate_;

const raw_ptr<Seat, ExperimentalAsh> seat_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class SecurityDelegateBindingTest : public test::WaylandServerTest {
ASSERT_NE(server_security_delegate_, nullptr);
}

raw_ptr<SecurityDelegate, ExperimentalAsh> server_security_delegate_ =
nullptr;
raw_ptr<SecurityDelegate, DanglingUntriaged | ExperimentalAsh>
server_security_delegate_ = nullptr;
};

TEST_F(SecurityDelegateBindingTest, ShellSurfaceHasSecurityDelegate) {
Expand Down
2 changes: 1 addition & 1 deletion components/exo/wayland/serial_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class SerialTracker {
absl::optional<EventType> GetEventType(uint32_t serial) const;

private:
raw_ptr<struct wl_display, ExperimentalAsh> display_;
raw_ptr<struct wl_display, DanglingUntriaged | ExperimentalAsh> display_;

// EventTypes are stored in a circular buffer, because serial numbers are
// issued sequentially and we only want to store the most recent events.
Expand Down
7 changes: 4 additions & 3 deletions components/exo/wayland/surface_augmenter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ class ClientData : public test::TestClient::CustomData {
std::unique_ptr<wl_surface> child_wl_surface;
std::unique_ptr<wl_subsurface> child_wl_subsurface;

raw_ptr<augmented_surface, ExperimentalAsh> augmented_surface = nullptr;
raw_ptr<augmented_sub_surface, ExperimentalAsh> augmented_sub_surface =
nullptr;
raw_ptr<augmented_surface, DanglingUntriaged | ExperimentalAsh>
augmented_surface = nullptr;
raw_ptr<augmented_sub_surface, DanglingUntriaged | ExperimentalAsh>
augmented_sub_surface = nullptr;
};

using SurfaceAugmenterTest = test::WaylandServerTest;
Expand Down
2 changes: 1 addition & 1 deletion components/exo/wayland/wayland_aura_shell_server_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class WaylandAuraShellServerTest : public test::WaylandServerTest {
surface_key);
}

raw_ptr<Display, ExperimentalAsh> display_;
raw_ptr<Display, DanglingUntriaged | ExperimentalAsh> display_;
};

// Home screen -> any window
Expand Down
6 changes: 4 additions & 2 deletions components/exo/wayland/wayland_display_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,12 @@ class WaylandDisplayHandler : public display::DisplayObserver,
raw_ptr<WaylandDisplayOutput, ExperimentalAsh> output_;

// The output resource associated with the display.
const raw_ptr<wl_resource, ExperimentalAsh> output_resource_;
const raw_ptr<wl_resource, DanglingUntriaged | ExperimentalAsh>
output_resource_;

// Resource associated with a zxdg_output_v1 object.
raw_ptr<wl_resource, ExperimentalAsh> xdg_output_resource_ = nullptr;
raw_ptr<wl_resource, DanglingUntriaged | ExperimentalAsh>
xdg_output_resource_ = nullptr;

// The listener is notified when the server-side client destruction begins.
ClientDestroyListener client_destroy_listener_;
Expand Down
14 changes: 9 additions & 5 deletions components/exo/wayland/wayland_display_observer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,15 @@ class WaylandDisplayObserverTest : public test::ExoTestBase {
}

int fds_[2] = {0, 0};
raw_ptr<wl_display, ExperimentalAsh> wayland_display_ = nullptr;
raw_ptr<wl_client, ExperimentalAsh> client_ = nullptr;
raw_ptr<wl_resource, ExperimentalAsh> aura_output_manager_resource_ = nullptr;
raw_ptr<wl_resource, ExperimentalAsh> wl_output_resource_ = nullptr;
raw_ptr<wl_resource, ExperimentalAsh> xdg_output_resource_ = nullptr;
raw_ptr<wl_display, DanglingUntriaged | ExperimentalAsh> wayland_display_ =
nullptr;
raw_ptr<wl_client, DanglingUntriaged | ExperimentalAsh> client_ = nullptr;
raw_ptr<wl_resource, DanglingUntriaged | ExperimentalAsh>
aura_output_manager_resource_ = nullptr;
raw_ptr<wl_resource, DanglingUntriaged | ExperimentalAsh>
wl_output_resource_ = nullptr;
raw_ptr<wl_resource, DanglingUntriaged | ExperimentalAsh>
xdg_output_resource_ = nullptr;
std::unique_ptr<WaylandDisplayOutput> output_;
std::unique_ptr<MockWaylandDisplayHandler> handler_;
};
Expand Down
2 changes: 1 addition & 1 deletion components/exo/wayland/wayland_display_output.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class WaylandDisplayOutput {

private:
const int64_t id_;
raw_ptr<wl_global, ExperimentalAsh> global_ = nullptr;
raw_ptr<wl_global, DanglingUntriaged | ExperimentalAsh> global_ = nullptr;
base::flat_map<wl_client*, wl_resource*> output_ids_;
bool had_registered_output_ = false;
bool is_destructing_ = false;
Expand Down
2 changes: 1 addition & 1 deletion components/exo/wayland/wayland_display_output_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class WaylandDisplayOutputTest : public test::WaylandServerTest {
TEST_F(WaylandDisplayOutputTest, DelayedSelfDestruct) {
class ClientData : public test::TestClient::CustomData {
public:
raw_ptr<wl_output, ExperimentalAsh> output = nullptr;
raw_ptr<wl_output, DanglingUntriaged | ExperimentalAsh> output = nullptr;
};

// Start with 2 displays.
Expand Down
3 changes: 2 additions & 1 deletion components/exo/wayland/xdg_shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,8 @@ class WaylandToplevel : public aura::WindowObserver {
}

const raw_ptr<wl_resource, ExperimentalAsh> xdg_toplevel_resource_;
const raw_ptr<wl_resource, ExperimentalAsh> xdg_surface_resource_;
const raw_ptr<wl_resource, DanglingUntriaged | ExperimentalAsh>
xdg_surface_resource_;
raw_ptr<WaylandXdgSurface, ExperimentalAsh> shell_surface_data_;
base::WeakPtrFactory<WaylandToplevel> weak_ptr_factory_{this};
};
Expand Down
5 changes: 3 additions & 2 deletions components/exo/wayland/zaura_shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,11 @@ class AuraToplevel {
ash::FocusCycler::Direction direction,
bool restart);

raw_ptr<ShellSurface, ExperimentalAsh> shell_surface_;
raw_ptr<ShellSurface, DanglingUntriaged | ExperimentalAsh> shell_surface_;
const raw_ptr<SerialTracker, ExperimentalAsh> serial_tracker_;
const raw_ptr<SerialTracker, ExperimentalAsh> rotation_serial_tracker_;
raw_ptr<wl_resource, ExperimentalAsh> xdg_toplevel_resource_;
raw_ptr<wl_resource, DanglingUntriaged | ExperimentalAsh>
xdg_toplevel_resource_;
raw_ptr<wl_resource, ExperimentalAsh> aura_toplevel_resource_;
bool supports_window_bounds_ = false;

Expand Down
2 changes: 1 addition & 1 deletion components/services/app_service/public/cpp/instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class Instance {
// The unique id for instance.
base::UnguessableToken instance_id_;

raw_ptr<aura::Window, ExperimentalAsh> window_ = nullptr;
raw_ptr<aura::Window, DanglingUntriaged | ExperimentalAsh> window_ = nullptr;

std::string launch_id_;
InstanceState state_ = InstanceState::kUnknown;
Expand Down
3 changes: 2 additions & 1 deletion components/soda/soda_installer_impl_chromeos_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ class SodaInstallerImplChromeOSTest : public testing::Test {
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
std::unique_ptr<SodaInstallerImplChromeOS> soda_installer_impl_;
std::unique_ptr<TestingPrefServiceSimple> pref_service_;
raw_ptr<ash::FakeDlcserviceClient, ExperimentalAsh> fake_dlcservice_client_;
raw_ptr<ash::FakeDlcserviceClient, DanglingUntriaged | ExperimentalAsh>
fake_dlcservice_client_;
base::test::ScopedFeatureList scoped_feature_list_;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,7 @@ class PrefServiceSyncableChromeOsTest : public testing::Test {

protected:
scoped_refptr<PrefRegistrySyncable> pref_registry_;
raw_ptr<PrefNotifierImpl, ExperimentalAsh>
raw_ptr<PrefNotifierImpl, DanglingUntriaged | ExperimentalAsh>
pref_notifier_; // Owned by |prefs_|.
scoped_refptr<TestingPrefStore> user_prefs_;
scoped_refptr<TestingPrefStore> standalone_browser_prefs_;
Expand Down
3 changes: 2 additions & 1 deletion components/user_manager/known_user_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ class KnownUserTest : public testing::Test {
base::test::TaskEnvironment::MainThreadType::UI};

// Owned by |scoped_user_manager_|.
raw_ptr<FakeUserManager, ExperimentalAsh> fake_user_manager_ = nullptr;
raw_ptr<FakeUserManager, DanglingUntriaged | ExperimentalAsh>
fake_user_manager_ = nullptr;
std::unique_ptr<ScopedUserManager> scoped_user_manager_;
TestingPrefServiceSimple local_state_;
};
Expand Down
2 changes: 1 addition & 1 deletion components/user_manager/user_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ TEST(UserTest, DeviceLocalAccountAffiliation) {
bool IsAffiliated() const { return user_ && user_->IsAffiliated(); }

private:
const raw_ptr<const User, ExperimentalAsh> user_;
const raw_ptr<const User, DanglingUntriaged | ExperimentalAsh> user_;
};

const AccountId account_id = AccountId::FromUserEmailGaiaId(kEmail, kGaiaId);
Expand Down

0 comments on commit a4ef6ce

Please sign in to comment.