[iOS] Defer MetricKit subscriber registration except in safe mode [chromium/src : main]

0 views

Skip to first unread message

Justin Novosad (Gerrit)

unread,

Aug 4, 2026, 12:38:06 PM (19 hours ago) Aug 4

to Olivier Robin, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Olivier Robin

Justin Novosad added 1 comment

File ios/chrome/app/application_delegate/metric_kit_subscriber.mm

Line 198, Patchset 6: base::SequencedTaskRunner::GetCurrentDefault()->PostDelayedTask(

Olivier Robin . resolved

should we use a main controller low priority task instead of handling this here?

Justin Novosad

Thanks for mentioning this. I did not know it existed. I like that it ensures the spreading out of deferred startup tasks.

Done.

Open in Gerrit

Related details

Attention is currently required from:

Olivier Robin

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Olivier Robin (Gerrit)

unread,

6:06 AM (1 hour ago) 6:06 AM

to Justin Novosad, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Justin Novosad

Olivier Robin voted and added 4 comments

Votes added by Olivier Robin

Code-Review

4 comments

File ios/chrome/app/application_delegate/metrics_mediator.mm

Line 612, Patchset 10 (Latest): [SafeModeCoordinator shouldStart] ||

Olivier Robin . unresolved

probably not for this CL:

I wonder how crash uploading works in safe mode.
It seems that safemode only checks crashpad reports [1]
Will the MetricKit report still be uploaded if they are received after safe mode starts?

[1] https://source.chromium.org/chromium/chromium/src/+/main:ios/chrome/browser/safe_mode/ui_bundled/safe_mode_view_controller.mm;drc=23f5271fa18d22362f3c739c631ef83ac709a1c1;l=112?q=hasReportToUpload&ss=chromium

Line 613, Patchset 10 (Latest): !IsMetrickitDeferRegistrationEnabled()) {

Olivier Robin . unresolved

if you plan to do a start_active=false experiment, it may be better to test the flag earlier in the chain, to catch the safemode case in the experiment. Otherwise safe mode startup will be excluded.
I would probably even put the flag first here.

File ios/chrome/app/application_delegate/metrics_mediator_unittest.mm

Line 319, Patchset 10 (Latest): id subscriber_mock = [OCMockObject mockForClass:[MetricKitSubscriber class]];

Olivier Robin . unresolved

nit:
prefer using v3 syntax

OCMStrictClassMock([MetricKitSubscriber class]);
OCMStub(subscriber_mock).andReturn(...);

https://ocmock.org/ocmock3/

Line 333, Patchset 10 (Latest):// Test that MetricKit registration happens immediately on user triggered

Olivier Robin . unresolved

Is this a requirement?
If the user enable metrics in the 5 seconds of the session, is it critical to upload metrickit reports immediately or could they just be deferred?

Open in Gerrit

Related details

Attention is currently required from:

Justin Novosad

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages