Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(25)

Issue 2721713002: [sync] Add typed url sync metadata to the history db (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 months, 3 weeks ago by Gang Wu
Modified:
7 months ago
Reviewers:
brettw, pavely
CC:
chromium-reviews, sky
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[sync] Add typed url sync metadata to the history db Two tables of sync metadata, are now being added to the history database. These two types are the per-entry EntityMetadata and the ModelTypeState, which is global state for the typed url model type. This change adds a migration to add the tables, and a new class TypedURLSyncMetadataDatabase wich would read/write/delete metadatas, with associated tests. Also a key word "AUTOINCREMENT" is added into urls table's column 'id' which is primary key. AUTOINCREMENT can avoid to reuse id number when an url got deleted and then recycled. BUG=696701 Review-Url: https://codereview.chromium.org/2721713002 Cr-Commit-Position: refs/heads/master@{#458232} Committed: https://chromium.googlesource.com/chromium/src/+/e8cbd06453b8b4dfc01fa61d004fa0469f42bd98

Patch Set 1 #

Patch Set 2 : DEPS issue #

Total comments: 2

Patch Set 3 : Pavely review #

Total comments: 22

Patch Set 4 : sky review #

Patch Set 5 : fix test issue caused by removing favicon #

Patch Set 6 : git rebase #

Total comments: 16

Patch Set 7 : brettw review #

Total comments: 4

Patch Set 8 : move meta table into typed_url_metadata_database #

Total comments: 8

Patch Set 9 : update for comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+581 lines, -71 lines) Patch
M components/history/core/browser/BUILD.gn View 1 3 chunks +4 lines, -0 lines 0 comments Download
M components/history/core/browser/history_database.h View 1 2 3 4 5 6 7 8 3 chunks +7 lines, -1 line 0 comments Download
M components/history/core/browser/history_database.cc View 1 2 3 4 5 6 7 8 5 chunks +21 lines, -2 lines 0 comments Download
M components/history/core/browser/in_memory_database.cc View 1 2 3 4 1 chunk +4 lines, -5 lines 0 comments Download
A components/history/core/browser/typed_url_sync_metadata_database.h View 1 2 3 4 5 6 7 8 1 chunk +74 lines, -0 lines 0 comments Download
A components/history/core/browser/typed_url_sync_metadata_database.cc View 1 2 3 4 5 6 7 8 1 chunk +140 lines, -0 lines 0 comments Download
A components/history/core/browser/typed_url_sync_metadata_database_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +143 lines, -0 lines 0 comments Download
M components/history/core/browser/url_database.h View 1 chunk +3 lines, -0 lines 0 comments Download
M components/history/core/browser/url_database.cc View 1 2 3 4 5 6 7 8 3 chunks +34 lines, -24 lines 0 comments Download
M components/history/core/browser/url_database_unittest.cc View 1 2 3 4 5 2 chunks +113 lines, -0 lines 0 comments Download
M components/test/data/omnibox/in_memory_url_index_test.db.txt View 1 2 3 4 2 chunks +37 lines, -38 lines 0 comments Download
M components/test/data/omnibox/in_memory_url_index_test_limited.db.txt View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 78 (62 generated)
Gang Wu
PTAL
7 months, 3 weeks ago (2017-02-28 00:12:45 UTC) #15
pavely
lgtm https://codereview.chromium.org/2721713002/diff/40001/components/history/core/browser/DEPS File components/history/core/browser/DEPS (right): https://codereview.chromium.org/2721713002/diff/40001/components/history/core/browser/DEPS#newcode3 components/history/core/browser/DEPS:3: "+components/sync", Did you add this because of warning ...
7 months, 2 weeks ago (2017-02-28 22:46:04 UTC) #18
Gang Wu
@sky, PTAL https://codereview.chromium.org/2721713002/diff/40001/components/history/core/browser/DEPS File components/history/core/browser/DEPS (right): https://codereview.chromium.org/2721713002/diff/40001/components/history/core/browser/DEPS#newcode3 components/history/core/browser/DEPS:3: "+components/sync", On 2017/02/28 22:46:03, pavely wrote: > ...
7 months, 2 weeks ago (2017-03-01 00:58:13 UTC) #20
sky
https://codereview.chromium.org/2721713002/diff/60001/components/history/core/browser/typed_url_sync_metadata_database.cc File components/history/core/browser/typed_url_sync_metadata_database.cc (right): https://codereview.chromium.org/2721713002/diff/60001/components/history/core/browser/typed_url_sync_metadata_database.cc#newcode1 components/history/core/browser/typed_url_sync_metadata_database.cc:1: // Copyright (c) 2017 The Chromium Authors. All rights ...
7 months, 2 weeks ago (2017-03-01 04:15:24 UTC) #25
Gang Wu
updated, PTAL https://codereview.chromium.org/2721713002/diff/60001/components/history/core/browser/typed_url_sync_metadata_database.cc File components/history/core/browser/typed_url_sync_metadata_database.cc (right): https://codereview.chromium.org/2721713002/diff/60001/components/history/core/browser/typed_url_sync_metadata_database.cc#newcode1 components/history/core/browser/typed_url_sync_metadata_database.cc:1: // Copyright (c) 2017 The Chromium Authors. ...
7 months, 2 weeks ago (2017-03-02 05:41:12 UTC) #37
sky
I'm going to kick this over to Brett as he is going to be handling ...
7 months, 2 weeks ago (2017-03-02 18:18:23 UTC) #42
brettw
https://codereview.chromium.org/2721713002/diff/180001/components/history/core/browser/typed_url_sync_metadata_database.cc File components/history/core/browser/typed_url_sync_metadata_database.cc (right): https://codereview.chromium.org/2721713002/diff/180001/components/history/core/browser/typed_url_sync_metadata_database.cc#newcode14 components/history/core/browser/typed_url_sync_metadata_database.cc:14: // storage_key id in urls talbe, used by service ...
7 months, 1 week ago (2017-03-13 19:58:59 UTC) #48
Gang Wu
updated https://codereview.chromium.org/2721713002/diff/180001/components/history/core/browser/typed_url_sync_metadata_database.cc File components/history/core/browser/typed_url_sync_metadata_database.cc (right): https://codereview.chromium.org/2721713002/diff/180001/components/history/core/browser/typed_url_sync_metadata_database.cc#newcode14 components/history/core/browser/typed_url_sync_metadata_database.cc:14: // storage_key id in urls talbe, used by ...
7 months, 1 week ago (2017-03-14 01:18:49 UTC) #53
brettw
https://codereview.chromium.org/2721713002/diff/180001/components/history/core/browser/typed_url_sync_metadata_database.cc File components/history/core/browser/typed_url_sync_metadata_database.cc (right): https://codereview.chromium.org/2721713002/diff/180001/components/history/core/browser/typed_url_sync_metadata_database.cc#newcode20 components/history/core/browser/typed_url_sync_metadata_database.cc:20: // id should inly one record in the table ...
7 months ago (2017-03-14 19:24:43 UTC) #54
Gang Wu
moved meta table. https://codereview.chromium.org/2721713002/diff/180001/components/history/core/browser/typed_url_sync_metadata_database.cc File components/history/core/browser/typed_url_sync_metadata_database.cc (right): https://codereview.chromium.org/2721713002/diff/180001/components/history/core/browser/typed_url_sync_metadata_database.cc#newcode20 components/history/core/browser/typed_url_sync_metadata_database.cc:20: // id should inly one record ...
7 months ago (2017-03-15 08:02:43 UTC) #57
Gang Wu
ping
7 months ago (2017-03-17 18:12:49 UTC) #60
brettw
I think I'd like to take a final look before checkin. But since this has ...
7 months ago (2017-03-20 17:27:06 UTC) #61
Gang Wu
updated, PTAL https://codereview.chromium.org/2721713002/diff/200001/components/history/core/browser/typed_url_sync_metadata_database.cc File components/history/core/browser/typed_url_sync_metadata_database.cc (right): https://codereview.chromium.org/2721713002/diff/200001/components/history/core/browser/typed_url_sync_metadata_database.cc#newcode33 components/history/core/browser/typed_url_sync_metadata_database.cc:33: for (const auto& storayge_key_to_metadata : metadata_records_map) On ...
7 months ago (2017-03-20 21:34:48 UTC) #69
brettw
lgtm
7 months ago (2017-03-20 21:46:24 UTC) #70
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2721713002/260001
7 months ago (2017-03-20 23:14:31 UTC) #75
commit-bot: I haz the power
7 months ago (2017-03-20 23:26:00 UTC) #78
Message was sent while issue was closed.
Committed patchset #9 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/e8cbd06453b8b4dfc01fa61d004f...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa