Shunya Shishido (Gerrit)

unread,

Jun 18, 2026, 2:18:31 AM (6 days ago) Jun 18

to Yoshisato Yanagisawa, Rakina Zata Amni, Chromium Metrics Reviews, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, James Maclean, asvitkine...@chromium.org

Attention needed from Rakina Zata Amni and Yoshisato Yanagisawa

New activity on the change

Open in Gerrit

Related details

Attention is currently required from:

Rakina Zata Amni
Yoshisato Yanagisawa

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

Yoshisato Yanagisawa (Gerrit)

unread,

Jun 19, 2026, 4:24:24 AM (4 days ago) Jun 19

to Shunya Shishido, Rakina Zata Amni, Chromium Metrics Reviews, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, James Maclean, asvitkine...@chromium.org

Attention needed from Rakina Zata Amni and Shunya Shishido

Yoshisato Yanagisawa voted and added 1 comment

Votes added by Yoshisato Yanagisawa

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Yoshisato Yanagisawa . resolved

non owner lgtm.
I am not sure this is the valid way to set DB filename. Let me delegate the check to the owner.

Open in Gerrit

Related details

Attention is currently required from:

Rakina Zata Amni
Shunya Shishido

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

Rakina Zata Amni (Gerrit)

unread,

Jun 22, 2026, 3:32:00 AM (yesterday) Jun 22

to Shunya Shishido, Yoshisato Yanagisawa, Chromium Metrics Reviews, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, James Maclean, asvitkine...@chromium.org

Attention needed from Shunya Shishido

Rakina Zata Amni added 1 comment

File content/browser/storage_partition_impl_unittest.cc

Line 2897, Patchset 6 (Latest):


  // 3. Clear data with a filter (should be a no-op / skip because DPO doesn't
  // support selective filter):

Rakina Zata Amni . unresolved

Is this ok and documented somewhere? (not really sure how these types of privacy-related deletions should work)

Open in Gerrit

Related details

Attention is currently required from:

Shunya Shishido

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

Shunya Shishido (Gerrit)

unread,

12:27 AM (16 hours ago) 12:27 AM

to Yoshisato Yanagisawa, Rakina Zata Amni, Chromium Metrics Reviews, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, James Maclean, asvitkine...@chromium.org

Attention needed from Rakina Zata Amni

Shunya Shishido added 1 comment

File content/browser/storage_partition_impl_unittest.cc

Line 2897, Patchset 6:


  // 3. Clear data with a filter (should be a no-op / skip because DPO doesn't
  // support selective filter):

Rakina Zata Amni . resolved

Is this ok and documented somewhere? (not really sure how these types of privacy-related deletions should work)

Shunya Shishido

Done. We should support filters. I misunderstood how we're using filters to clear data. Added ClearDataWithFilter() and updated the test.

Open in Gerrit

Related details

Attention is currently required from:

Rakina Zata Amni

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

Rakina Zata Amni (Gerrit)

unread,

2:32 AM (13 hours ago) 2:32 AM

to Shunya Shishido, Yoshisato Yanagisawa, Chromium Metrics Reviews, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, James Maclean, asvitkine...@chromium.org

Attention needed from Shunya Shishido

Rakina Zata Amni added 1 comment

File content/browser/declarative_performance_observer/declarative_performance_observer_store.cc

Line 574, Patchset 7 (Latest): for (const auto& origin : cached_policies_) {

Rakina Zata Amni . unresolved

Because `cached_policies_` might be empty (or incomplete) while `!loaded_` is true, iterating over it won't catch origins that are still being loaded from the database.

If a filter matches an origin that is in the middle of loading:
1. It won't be found in `cached_policies_`.
2. It won't be added to `to_remove`.
3. It won't be added to `modified_during_load_`.
4. When `OnPoliciesLoadedOnUISequence` finally runs, it will incorrectly insert the origin into `cached_policies_` because it doesn't know it was supposed to be filtered out (even though the database deletion task will correctly delete it from disk).

To fix this race condition, you'll need to keep track of filters applied during load (e.g., in a `std::vector<OriginMatcherFunction> pending_filters_` member) when `!loaded_` is true. Then, in `OnPoliciesLoadedOnUISequence`, apply these pending filters to the `loaded` vector to drop matching origins before inserting the rest into `cached_policies_`.

Open in Gerrit

Related details

Attention is currently required from:

Shunya Shishido

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

Shunya Shishido (Gerrit)

unread,

2:56 AM (13 hours ago) 2:56 AM

to Yoshisato Yanagisawa, Rakina Zata Amni, Chromium Metrics Reviews, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, James Maclean, asvitkine...@chromium.org

Attention needed from Rakina Zata Amni

Shunya Shishido added 1 comment

File content/browser/declarative_performance_observer/declarative_performance_observer_store.cc

Line 574, Patchset 7: for (const auto& origin : cached_policies_) {

Rakina Zata Amni . resolved

Because `cached_policies_` might be empty (or incomplete) while `!loaded_` is true, iterating over it won't catch origins that are still being loaded from the database.
If a filter matches an origin that is in the middle of loading:
1. It won't be found in `cached_policies_`.
2. It won't be added to `to_remove`.
3. It won't be added to `modified_during_load_`.
4. When `OnPoliciesLoadedOnUISequence` finally runs, it will incorrectly insert the origin into `cached_policies_` because it doesn't know it was supposed to be filtered out (even though the database deletion task will correctly delete it from disk).
To fix this race condition, you'll need to keep track of filters applied during load (e.g., in a `std::vector<OriginMatcherFunction> pending_filters_` member) when `!loaded_` is true. Then, in `OnPoliciesLoadedOnUISequence`, apply these pending filters to the `loaded` vector to drop matching origins before inserting the rest into `cached_policies_`.

Shunya Shishido

Done

Open in Gerrit

Related details

Attention is currently required from:

Rakina Zata Amni

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

Shunya Shishido (Gerrit)

unread,

3:59 AM (12 hours ago) 3:59 AM

to Christian Dullweber, Yoshisato Yanagisawa, Rakina Zata Amni, Chromium Metrics Reviews, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, James Maclean, asvitkine...@chromium.org

Attention needed from Christian Dullweber and Rakina Zata Amni

Shunya Shishido added 1 comment

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Shunya Shishido . resolved

dullweber: Could you give an owner review to t/m/h/m/h/enums.xml? Thank you!

Open in Gerrit

Related details

Attention is currently required from:

Christian Dullweber
Rakina Zata Amni

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

Christian Dullweber (Gerrit)

unread,

4:47 AM (11 hours ago) 4:47 AM

to Shunya Shishido, Yoshisato Yanagisawa, Rakina Zata Amni, Chromium Metrics Reviews, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, James Maclean, asvitkine...@chromium.org

Attention needed from Rakina Zata Amni and Shunya Shishido

Christian Dullweber voted and added 1 comment

Votes added by Christian Dullweber

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Christian Dullweber . resolved

enums.xml lgtm

Open in Gerrit

Related details

Attention is currently required from:

Rakina Zata Amni
Shunya Shishido

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

Reply all

Reply to author

Forward

Connect DeclarativePerformanceObserverStore to StoragePartitionImpl [chromium/src : main]

Shunya Shishido (Gerrit)

New activity on the change

Related details

Yoshisato Yanagisawa (Gerrit)

Yoshisato Yanagisawa voted and added 1 comment

Votes added by Yoshisato Yanagisawa

1 comment

Related details

Rakina Zata Amni (Gerrit)

Rakina Zata Amni added 1 comment

Related details

Shunya Shishido (Gerrit)

Shunya Shishido added 1 comment

Related details

Rakina Zata Amni (Gerrit)

Rakina Zata Amni added 1 comment

Related details

Shunya Shishido (Gerrit)

Shunya Shishido added 1 comment

Related details

Shunya Shishido (Gerrit)

Shunya Shishido added 1 comment

Related details

Christian Dullweber (Gerrit)

Christian Dullweber voted and added 1 comment

Votes added by Christian Dullweber

1 comment

Related details