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

Issue 675794 link

Starred by 1 user
Status: Fixed
Owner:
█ twif...@chromium.org
Last visit > 30 days ago
Closed: Dec 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

PrecacheManagerFactory should contain DependsOn() statements.

Project Member Reported by twif...@chromium.org, Dec 20 2016 
As a KeyedServiceFactory, it should establish its dependence on other KSFs, but it is lacking those declarations. To my knowledge, this hasn't bitten us in production, but it may in the future. Or, it perhaps it is responsible for some of our lack of coverage?
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 20 2016 

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

commit 44dbf999f94cfba6c40ceadd4f4ed9b5f4a1eca2
Author: twifkak <twifkak@chromium.org>
Date: Tue Dec 20 01:26:29 2016

Add DependsOn() calls to PrecacheManagerFactory.

Add dependencies on ProfileSyncServiceFactory, HistoryServiceFactory,
and DataReductionProxyChromeSettingsFactory, to match the usage in
BuildServiceInstanceFor.

BUG= 675794 

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

[modify] https://crrev.com/44dbf999f94cfba6c40ceadd4f4ed9b5f4a1eca2/chrome/browser/precache/precache_manager_factory.cc

Comment 2 by twif...@chromium.org, Dec 20 2016

Status: Fixed (was: Started)
OK, it looks like DependsOn is used for two things:

1. Construction order of services at context creation time, but only those for which ServiceIsCreatedWithContext() is true. None of these factories meet that criterion.

2. Destruction order of all services, but Chrome Android never gracefully shuts down anyway.

So this isn't worth merging.
►

Sign in to add a comment