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

Issue 793917 link

Starred by 1 user
Status: Fixed
Owner:
█ holte@chromium.org
Last visit > 30 days ago
Closed: Apr 2018
Cc:
skare@chromium.org
isherman@chromium.org
ios-bugs@chromium.org
asvitk...@chromium.org
█ rkaplow@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

IsHistorySyncEnabledOnAllProfiles() API name is misleading, since it doesn't mention custom passphrase that it also checks

Project Member Reported by asvitk...@chromium.org, Dec 11 2017 
IsHistorySyncEnabledOnAllProfiles() API name is misleading, since it doesn't mention custom passphrase that it also checks.

The implementation does appear to check it, but this isn't documented in either method name or its contents. Nor in the API of the helper class it uses.

https://cs.chromium.org/chromium/src/components/ukm/observers/sync_disable_observer.h?rcl=d270641eacb0e6d0c3ba76d8696ad4910878bd07&l=29

I suggest changing the name to mention it so that things are clear.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 23 2018 

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

commit 0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4
Author: Steven Holte <holte@google.com>
Date: Mon Apr 23 21:53:47 2018

Rename IsHistorySyncEnabledOnAllProfiles

Bug:  793917 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I80bcead4276b5be658588d5b32ab09ebf9e952dc
Reviewed-on: https://chromium-review.googlesource.com/1008406
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552845}
[modify] https://crrev.com/0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4/chrome/browser/metrics/chrome_metrics_service_client.h
[modify] https://crrev.com/0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4/components/metrics/metrics_service_client.cc
[modify] https://crrev.com/0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4/components/metrics/metrics_service_client.h
[modify] https://crrev.com/0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4/components/metrics_services_manager/metrics_services_manager.cc
[modify] https://crrev.com/0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4/components/ukm/observers/sync_disable_observer.cc
[modify] https://crrev.com/0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4/components/ukm/observers/sync_disable_observer.h
[modify] https://crrev.com/0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4/components/ukm/observers/sync_disable_observer_unittest.cc
[modify] https://crrev.com/0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4/ios/chrome/browser/metrics/ios_chrome_metrics_service_client.h
[modify] https://crrev.com/0221a1b1457faf2d86ad01cac4b0bc6944cfb6c4/ios/chrome/browser/metrics/ios_chrome_metrics_service_client.mm

Comment 2 by holte@chromium.org, Apr 27 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment