New issue
Advanced search Search tips

Issue 717443 link

Starred by 1 user
Status: Fixed
Owner:
dullweber@chromium.org
Closed: Jul 2017
Cc:
msramek@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Generalize sync-aware browsing data counters

Project Member Reported by dullweber@chromium.org, May 2 2017 
Currently the History-, Autofill-, and Passwordcounter contain code that detects whether the relevant data type is synced. This should be generalized with a common base class (SyncAwareBrowsingDataCounter?).

Also there is some duplication in the test cases. There should either be a common base test class for these counters or all sync behavior tests should be moved to a special test file. (maybe also solving http://crbug.com/553421).
Project Member

Comment 1 by bugdroid1@chromium.org, May 16 2017 

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

commit 52b329c5fe6e273189927ef663f56304a65ceef8
Author: dullweber <dullweber@chromium.org>
Date: Tue May 16 12:24:40 2017

Move browsing_data counter sync tests to separate file

The history, passwords and autofill counters currently each have a test to check if they behave correctly when different sets of datatypes are being synced.
This required the whole test class to be a SyncTest. This change moves these sync related tests to a separate test file, reverts the other tests to normal InProcessBrowserTests and removes all unnecessary helper methods from these tests.

BUG= 717443 

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

[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/chrome/browser/browsing_data/autofill_counter_browsertest.cc
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/chrome/browser/browsing_data/history_counter_browsertest.cc
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/chrome/browser/browsing_data/passwords_counter_browsertest.cc
[add] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/chrome/browser/browsing_data/sync_aware_counter_browsertest.cc
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/chrome/test/BUILD.gn
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/components/browsing_data/core/browsing_data_utils.cc
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/components/browsing_data/core/browsing_data_utils_unittest.cc
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/components/browsing_data/core/counters/autofill_counter.cc
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/components/browsing_data/core/counters/autofill_counter.h
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/components/browsing_data/core/counters/browsing_data_counter.cc
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/components/browsing_data/core/counters/browsing_data_counter.h
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/components/browsing_data/core/counters/history_counter.cc
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/components/browsing_data/core/counters/history_counter.h
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/components/browsing_data/core/counters/passwords_counter.cc
[modify] https://crrev.com/52b329c5fe6e273189927ef663f56304a65ceef8/components/browsing_data/core/counters/passwords_counter.h
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 19 2017 

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

commit a4158ccaaa06f38cd72d7c91d8c9c37451de5f72
Author: Christian Dullweber <dullweber@chromium.org>
Date: Wed Jul 19 09:50:30 2017

Add helper to track sync status for BrowsingDataCounters

This CL adds a class browsing_data::SyncTracker, that subscribes to
sync events and notifies a counter if its sync status changed.
This way code that is duplicated in history, autofill and passwords
counter can be removed.

Bug:  717443 
Change-Id: I7a04ba7170cf652414fd45a36711a2547c623a23
Reviewed-on: https://chromium-review.googlesource.com/575131
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487806}
[modify] https://crrev.com/a4158ccaaa06f38cd72d7c91d8c9c37451de5f72/components/browsing_data/core/BUILD.gn
[modify] https://crrev.com/a4158ccaaa06f38cd72d7c91d8c9c37451de5f72/components/browsing_data/core/counters/autofill_counter.cc
[modify] https://crrev.com/a4158ccaaa06f38cd72d7c91d8c9c37451de5f72/components/browsing_data/core/counters/autofill_counter.h
[modify] https://crrev.com/a4158ccaaa06f38cd72d7c91d8c9c37451de5f72/components/browsing_data/core/counters/history_counter.cc
[modify] https://crrev.com/a4158ccaaa06f38cd72d7c91d8c9c37451de5f72/components/browsing_data/core/counters/history_counter.h
[modify] https://crrev.com/a4158ccaaa06f38cd72d7c91d8c9c37451de5f72/components/browsing_data/core/counters/passwords_counter.cc
[modify] https://crrev.com/a4158ccaaa06f38cd72d7c91d8c9c37451de5f72/components/browsing_data/core/counters/passwords_counter.h
[add] https://crrev.com/a4158ccaaa06f38cd72d7c91d8c9c37451de5f72/components/browsing_data/core/counters/sync_tracker.cc
[add] https://crrev.com/a4158ccaaa06f38cd72d7c91d8c9c37451de5f72/components/browsing_data/core/counters/sync_tracker.h

Comment 3 by dullweber@chromium.org, Jul 19 2017

Status: Fixed (was: Available)
►

Sign in to add a comment