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

Issue 851869 link

Starred by 0 users
Status: Fixed
Owner:
jkrcal@chromium.org
Closed: Nov 23
Cc:
mastiz@chromium.org
jkrcal@chromium.org
mamir@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

TypedURLSyncBridge keeps metadata for entries with visits but no typed visit

Project Member Reported by jkrcal@chromium.org, Jun 12 2018 
Currently, there are inconsistencies between how the bridge defines its data (e.g. in GetData() and on other places) and what metadata it stores:
 - a data entry is an URL for which a _typed_ visit exists
 - it stores metadata for URLs for which _any_ visit exists.

The reason is technical, the bridge deals differently with explicit deletion and with implicit expiry. This bit gets currently only propagated to OnURLsDeleted() which gets triggered only when the last visit for an URL disappears. Thus, we delete metadata entries only when the last visit gets deleted.

The solution is to plumb this bit into OnURLsModified() as well and do the metadata deletion as soon as the last _typed_ visit gets deleted.

Comment 1 by jkrcal@chromium.org, Jun 12 2018 

This bug is also somewhat related to https://crbug.com/851867 that proposes to change the expiry logic.

Comment 2 by mastiz@chromium.org, Jul 4

Labels: sync-fixit-2018q3

Comment 3 by mastiz@chromium.org, Oct 24

Labels: sync-fixit-2018q4

Comment 4 by jkrcal@chromium.org, Nov 20

Owner: jkrcal@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 21 

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

commit 2f231240e80a0b96b2ef92c8576683babefe751a
Author: Jan Krcal <jkrcal@chromium.org>
Date: Wed Nov 21 11:06:19 2018

[History] Add expiry bit into HistoryBackendObserver::OnURLsModified()

This CL allow propagating is_from_expiration bool to TypedURLSyncBridge.
It is important for keeping sync data (in history DB) with sync metadata
consistent. This bool will be used in a follow-up CL.

Bug:  851869 
Change-Id: I96bec4ecc86f9357a82b7abf4ca7ee8320ef998d
Reviewed-on: https://chromium-review.googlesource.com/c/1343005
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609995}
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/chrome/browser/history/android/android_provider_backend.cc
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/chrome/browser/history/android/android_provider_backend_unittest.cc
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/components/history/core/browser/expire_history_backend.cc
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/components/history/core/browser/expire_history_backend_unittest.cc
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/components/history/core/browser/history_backend.cc
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/components/history/core/browser/history_backend.h
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/components/history/core/browser/history_backend_notifier.h
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/components/history/core/browser/history_backend_observer.h
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/components/history/core/browser/sync/typed_url_sync_bridge.cc
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/components/history/core/browser/sync/typed_url_sync_bridge.h
[modify] https://crrev.com/2f231240e80a0b96b2ef92c8576683babefe751a/components/history/core/browser/sync/typed_url_sync_bridge_unittest.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 22 

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

commit 2aea66f9ac8308d5d93914660a18177eeb0b8074
Author: Jan Krcal <jkrcal@chromium.org>
Date: Thu Nov 22 16:28:58 2018

[Typed URL Sync] Expire metadata when the last typed visit expires

This CL makes sure we expire local sync metadata as soon as the last
typed visit for a URL expires.

As a result, we have a (more) consistent sets of data (URLs with
non-expired typed visits as defined by bridge's GetData()) and metadata.

Bug:  851869 
Change-Id: I4f100ce8b4fc1281a0e7667e69db20f10ccea9c3
Reviewed-on: https://chromium-review.googlesource.com/c/1346398
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610435}
[modify] https://crrev.com/2aea66f9ac8308d5d93914660a18177eeb0b8074/chrome/browser/sync/test/integration/single_client_typed_urls_sync_test.cc
[modify] https://crrev.com/2aea66f9ac8308d5d93914660a18177eeb0b8074/chrome/browser/sync/test/integration/two_client_typed_urls_sync_test.cc
[modify] https://crrev.com/2aea66f9ac8308d5d93914660a18177eeb0b8074/chrome/browser/sync/test/integration/typed_urls_helper.cc
[modify] https://crrev.com/2aea66f9ac8308d5d93914660a18177eeb0b8074/chrome/browser/sync/test/integration/typed_urls_helper.h
[modify] https://crrev.com/2aea66f9ac8308d5d93914660a18177eeb0b8074/components/history/core/browser/sync/typed_url_sync_bridge.cc
[modify] https://crrev.com/2aea66f9ac8308d5d93914660a18177eeb0b8074/components/history/core/browser/sync/typed_url_sync_bridge.h
[modify] https://crrev.com/2aea66f9ac8308d5d93914660a18177eeb0b8074/components/history/core/browser/sync/typed_url_sync_bridge_unittest.cc

Comment 7 by jkrcal@chromium.org, Nov 23

Status: Fixed (was: Started)
►

Sign in to add a comment