There's some low- and medium- hanging fruit to be reaped in the UpgradeDetector, from resource usage and code health points of view. Here are some of my thoughts after spending some time adding a new consumer of its notifications and adding new features to it:

The UpgradeDetector notices when Chrome needs to be restarted for an update and broadcasts notifications about this to observers. Things I don't like about it as-is:

- [Windows,macOS,Linux]: It wakes the browser process up every 2h to see if the browser is "outdated", meaning it was built over 12 weeks ago. The build time is immutable, so it could instead featch the build time and network time in the bg sometime after startup and then schedule a wakeup once the delta reaches 12 weeks.

- [Windows]: It wakes the browser process up every 2h to see if Chrome has been updated and needs to be restarted. Rather than checking a value in the registry via polling, it should use base::win::RegKey::StartWatching to be woken by the OS only when values in the relevant key change (which is must less than once every 2h).

- [macOS,Linux]: Same polling as above -- I don't know enough about keystone_glue or Linux stuff to know if there's a way to get the needed signal without polling. If not, polling should be used only for these platforms (or in case StartWatching on Windows fails for some reason).

- [All]: Once an upgrade has been detected, the instance wakes up every 20 minutes to do little more than check the clock. This could easily be changed to wake up only when there's a change in state that observers would care about (e.g., moving from the "low" to the "elevated" annoyance level). Fewer process wakeups is good. Unfortunately, it's not obvious to me that none of the observers rely on this "every 20min" behavior. I think the system tray on Chrome OS and the app menu elsewhere are okay. I found that there is an event sent to extensions on each of these notifications -- I don't know if they rely on the polling (e.g., so that newly loaded extensions are told about the state on the next 20min wakeup).

- [All]: The detector is a singleton accessed via UpgradeDetector::GetInstance(). I'm of the mind that it should hang off of the BrowserProcess in such a way as to allow it to be replaced with a fake in browser tests. I also think it should be turned off during orderly shutdown on all platforms (it is already on Chrome OS).

- [All]: The implementation is split among UpgradeDetector, UpgradeDetectorImpl (Win,mac,lin), and UpgradeDetectorChromeos (ChromeOS). This makes the code harder to understand, harder to test across all platforms, and leads to duplication across the two subclasses. It would be a huge improvement to bring the core functionality into UpgradeDetector and keep the platform-specific code in delegates or somesuch. This would make it much easier to create comprehensive unit tests for the platform-specific code and for the general code.

- [Chrome OS]: The other platforms send out a "gotta relaunch!" notification if the variations service tells its observers that a "critical" change to experiment state has been received. I suppose this is so that we can handle OMG events where we need to shut off an experiment, for example. Chrome OS doesn't handle these events. It's not clear to me if this is by design or if it's an accident of the implementation split above.

- [All]: The observer interface and the various bits of state held in the detector have gotten pretty complicated over the years as a result of features being added to the detector (outdated builds, critical experiment state, critical updates). I wonder if everyting could be cleaned up by separating concerns -- maybe the critical experiment state thingie or the outdated build thingie should actually be in their own classes. Just a thought. Each time I look at UpgradeDetector::NotifyUpgrade and try to reason about the order of notifications, the fact that OnUpgradeRecommended is sent for every kind of notification, etc, I get the sense that too many things have been piled on here without enough thought.

I'm CCing a few folks who may have thoughts about these items, their importance, and whom might be given the job of addressing them.