Evaluate whether using IsSyncFeatureActive() is the right check inside DocumentProvider::IsDocumentProviderAllowed()
.
The right check might be "syncer::GetUploadToGoogleState(..) == ACTIVE".

Context:

-jdonnely@
Rather than remove this method, it seems like we should be using EnableGoogleServices() instead. Or is it the case now that sync being active guarantees that the user has accepted the Unity opt-in?

-tangltom@
Active sync doesn't guarantee opt into Unity. Even before the current change, Unity was in line with Sync-Everything rather than just sync being active.

After this change there is no more unified-consent-given, i.e. the API to check whether the user gave unified consent is removed.
Instead EnableGoogleServices() enables all the services once during opt-in and then doesn't care about any settings changes afterwards.

If you want to rely on a preference here, then you can add one for this feature and enable it in the UnifiedConsentService or add it to the UnifiedConsentServiceClient.

Also I'm not sure if IsSyncActive() is the right thing to check here.
Marc, could you weigh in?

Note: There is still a consent helper for url-keyed data collection if you were thinking about this https://cs.chromium.org/chromium/src/components/unified_consent/url_keyed_data_collection_consent_helper.h?type=cs&g=0.


-treib@
AFAIK Omnibox requests contain cookies i.e. are authenticated, so the UrlKeyed helper is probably not the right thing.

AutocompleteProviderClient::IsSyncActive maps to SyncService::IsSyncFeatureActive. That means:

The user opted in to Sync (though with no particular set of data types - they might all be disabled).
Sync is actually active (after browser startup, it usually takes ~10s to get into this state).

HOWEVER:

Sync may be paused (i.e. the user signed out of the content area).
There might be some auth error etc. that prevents Sync from actually syncing.
There might be a custom passphrase, which means we might not have consent to upload user data.

Overall, IsSyncFeatureActive *might* be the right thing here (I don't know what exact conditions are meant to be checked), but it's probably not. I think "syncer::GetUploadToGoogleState(..) == ACTIVE" would probably be better. (Depending on your exact needs, INITIALIZING might also be sufficient.)