New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 854465 link

Starred by 1 user
Status: Available
Owner: ----
Cc:
█ dschuyler@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug
Team-Security-UX



Sign in to add a comment

Refactor Settings webui tests to allow the list of content settings to be computed only once

Project Member Reported by patricia...@chromium.org, Jun 20 2018 
crrev.com/c/1098577 refactors the Site Settings web UI to recompute the list of content settings every time it is required. This slows down the original method by 0.6ms according to a pageload of Site Details, profiled in Chrome DevTools.

This could be updated to compute only once and immediately cache the results, but this would cause some automated tests that rely on being able to flip feature flags dynamically to fail. These tests can be refactored to prevent this from happening.

Comment 1 by patricia...@chromium.org, Jun 20 2018 

dschuyler: I think if performance is an issue, there will be a lot of other places with low-hanging fruits that will probably give us more than a 0.6ms improvement. If I'm interpreting the DevTools performance call tree correctly, in Site Details getCategoryList() takes 0.8ms compared to the previous 0.2ms. (I haven't measured the performance of your suggested change in https://chromium-review.googlesource.com/c/chromium/src/+/1098577/5/chrome/browser/resources/settings/site_settings/site_settings_behavior.js#230 yet, but I expect that number would be between 0.8 and 0.2ms).

Are you OK with leaving this open until All Sites is mostly done, then re-evaluate whether we need to optimise then?

Comment 2 by dschuyler@chromium.org, Jun 20 2018 

#1 Yep! SGTM.
►

Sign in to add a comment