New issue
Advanced search Search tips

Issue 598841 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Jul 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Quirks Post-release code cleanup

Project Member Reported by glevin@chromium.org, Mar 29 2016

Issue description

After the Quirks code has been in the wild for a release or two, there is some code cleanup that should be done:

1) Remove experimental flag #enable-quirks-client

2) Remove random delay for initial Quirks Server check.  See
https://code.google.com/p/chromium/codesearch#chromium/src/components/quirks/quirks_manager.cc&l=225

3) At present, if the server returns a 5XX error, the Client will recheck the server with exponential backoff.  Add code so that, after a certain amount of time or checks, we give up and mark that file as checked, so that we don't look again for 30 days.

4) There are still 7 icc files hard-coded into all Chromebooks at /usr/share/color/icc/.  Consider removing these files, and the code to look for them.  See
https://code.google.com/p/chromium/codesearch#chromium/src/components/quirks/quirks_manager.cc&l=49
 

Comment 1 by glevin@chromium.org, May 27 2016

Status: Started (was: Assigned)
Items (1) & (3) are under review at https://codereview.chromium.org/2016883003/

CL for Item (2) is at https://codereview.chromium.org/2006143011 , but it was decided to delay this until M-54, so I'm just sitting on it until then.

It was decided that hard-coded files in Item (4) can be left where they are, and no action is needed here.
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b207f7a5c9cd60d4c68fe207b9b8b1949c46362b

commit b207f7a5c9cd60d4c68fe207b9b8b1949c46362b
Author: glevin <glevin@chromium.org>
Date: Fri Jun 03 14:50:02 2016

Remove Quirks flag, limit server retries

BUG= 598841 
TEST=Check that disable-quirks-client is no longer on the experimental
flags list.

This removes the disable-quirks-client command line flag, and places a
limit of 10 on the number of times (with exponential backoff) that
Chrome OS will attempt to contact the Quirks server in one session.

Review-Url: https://codereview.chromium.org/2016883003
Cr-Commit-Position: refs/heads/master@{#397704}

[modify] https://crrev.com/b207f7a5c9cd60d4c68fe207b9b8b1949c46362b/ash/display/display_color_manager_chromeos_unittest.cc
[modify] https://crrev.com/b207f7a5c9cd60d4c68fe207b9b8b1949c46362b/chrome/app/chromeos_strings.grdp
[modify] https://crrev.com/b207f7a5c9cd60d4c68fe207b9b8b1949c46362b/chrome/browser/about_flags.cc
[modify] https://crrev.com/b207f7a5c9cd60d4c68fe207b9b8b1949c46362b/components/quirks.gypi
[modify] https://crrev.com/b207f7a5c9cd60d4c68fe207b9b8b1949c46362b/components/quirks/BUILD.gn
[modify] https://crrev.com/b207f7a5c9cd60d4c68fe207b9b8b1949c46362b/components/quirks/quirks_client.cc
[modify] https://crrev.com/b207f7a5c9cd60d4c68fe207b9b8b1949c46362b/components/quirks/quirks_manager.cc
[delete] https://crrev.com/6b4478c2f9878f15543c62119e39f1e4b424a156/components/quirks/switches.cc
[delete] https://crrev.com/6b4478c2f9878f15543c62119e39f1e4b424a156/components/quirks/switches.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 11 2016

Comment 4 by glevin@chromium.org, Jul 13 2016

Labels: -M-53 M-54
Status: Fixed (was: Started)
Status of items in initial report:

1) Remove #enable-quirks-client - Done (first CL, M-53)

2) Remove random delay - Done (second CL, M-54) (note: link no longer valid)

3) Limit server rechecks per session - Done (first CL, M-53)

4) Decision about 7 hard-coded files - 5 removed for bad data, 2 left as is

Comment 5 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 6 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 7 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 8 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 9 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment