Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-11 12:25:53 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1388884 - Do not delete bookmark tombstones r=rnewman

Browse Source 
As implemented, this means we might upload tombstones for never-synced
bookmarks. This _should_ be harmless.

MozReview-Commit-ID: DZx9yWDs1ie

--HG--
extra : rebase_source : d4abf10bded3f6ab39d13f0152cf981cd9fe4df7
This commit is contained in:
Grigory Kruglov 2017-08-29 14:01:14 -04:00
parent cd6f4e337e
commit 0074183cb7
3 changed files with 41 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
View File

@ -2388,12 +2388,6 @@ public class BrowserProvider extends SharedBrowserDatabaseProvider {
final List<String> guids = getBookmarkDescendantGUIDs(db, selection, selectionArgs);
changed += bulkDeleteByBookmarkGUIDs(db, uri, guids);
try {
cleanUpSomeDeletedRecords(uri, TABLE_BOOKMARKS);
} catch (Exception e) {
// We don't care.
Log.e(LOGTAG, "Unable to clean up deleted bookmark records: ", e);
}
return changed;
}

12
mobile/android/tests/background/junit4/src/org/mozilla/gecko/db/BrowserProviderBookmarksTest.java
View File

@ -372,11 +372,11 @@ public class BrowserProviderBookmarksTest {
assertVersionsForGuid(BrowserContract.Bookmarks.MOBILE_FOLDER_GUID, 2, 0);
// delete bookmark from fennec, since this record is not synced, it will be DELETED entirely
// delete bookmark from fennec, expect bookmark tombstones to persist for unsynced records
deleteByGuid(bookmarksTestUri, guid1);
assertEquals(0, getRowCount(Bookmarks.GUID + " = ?", new String[]{guid1}));
assertEquals(1, getRowCount(Bookmarks.GUID + " = ?", new String[]{guid1}));
assertEquals(9, getTotalNumberOfBookmarks());
assertEquals(10, getTotalNumberOfBookmarks());
assertVersionsForGuid(BrowserContract.Bookmarks.MOBILE_FOLDER_GUID, 3, 0);
@ -385,7 +385,7 @@ public class BrowserProviderBookmarksTest {
insert("bookmark-4", "https://www.mozilla-4.org", guid4,
rootId, BrowserContract.Bookmarks.TYPE_BOOKMARK, true);
assertEquals(10, getTotalNumberOfBookmarks());
assertEquals(11, getTotalNumberOfBookmarks());
// inserting bookmark from sync should not touch its parent's localVersion
assertVersionsForGuid(BrowserContract.Bookmarks.MOBILE_FOLDER_GUID, 3, 0);
@ -397,7 +397,7 @@ public class BrowserProviderBookmarksTest {
assertVersionsForGuid(BrowserContract.Bookmarks.MOBILE_FOLDER_GUID, 4, 0);
// sanity check our total number of bookmarks, it shouldn't have changed because of the delete
assertEquals(10, getTotalNumberOfBookmarks());
assertEquals(11, getTotalNumberOfBookmarks());
// assert that bookmark is still present after deletion, and its local version has been bumped
assertEquals(1, getRowCount(Bookmarks.GUID + " = ?", new String[]{guid4}));
@ -405,7 +405,7 @@ public class BrowserProviderBookmarksTest {
// delete bookmark from sync, will just DELETE it from the database entirely
deleteByGuid(bookmarksTestSyncUri, guid2);
assertEquals(9, getTotalNumberOfBookmarks());
assertEquals(10, getTotalNumberOfBookmarks());
assertEquals(0, getRowCount(Bookmarks.GUID + " = ?", new String[]{guid2}));
// deleting bookmark from sync should not touch its parent's localVersion

64
mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBrowserProvider.java
View File

@ -675,35 +675,7 @@ public class testBrowserProvider extends ContentProviderTest {
return id;
}
@Override
public void test() throws Exception {
// Test that unsynced bookmarks are dropped from the database.
long id = insertOneBookmark();
int changed = mProvider.delete(BrowserContract.Bookmarks.CONTENT_URI,
BrowserContract.Bookmarks._ID + " = ?",
new String[] { String.valueOf(id) });
// Deletions also affect parents of folders, and so that must be accounted for.
mAsserter.is((changed == 2), true, "Inserted bookmark was deleted");
Cursor c = getBookmarkById(appendUriParam(BrowserContract.Bookmarks.CONTENT_URI, BrowserContract.PARAM_SHOW_DELETED, "1"), id);
mAsserter.is(c.moveToFirst(), false, "Unsynced deleted bookmark was dropped from the database");
// Test that synced bookmarks are only marked as deleted.
id = insertOneBookmark("test-guid", true);
// Bookmark has been inserted from sync. Let's delete it again, and test that it has not
// been dropped from the database.
changed = mProvider.delete(BrowserContract.Bookmarks.CONTENT_URI,
BrowserContract.Bookmarks._ID + " = ?",
new String[] { String.valueOf(id) });
// Deletions also affect parents of folders, and so that must be accounted for.
mAsserter.is((changed == 2), true, "Inserted bookmark was deleted");
c = getBookmarkById(appendUriParam(BrowserContract.Bookmarks.CONTENT_URI, BrowserContract.PARAM_SHOW_DELETED, "1"), id);
mAsserter.is(c.moveToFirst(), true, "Deleted bookmark was only marked as deleted");
private void verifyMarkedAsDeleted(Cursor c) {
mAsserter.is(c.getString(c.getColumnIndex(BrowserContract.Bookmarks.TITLE)), null,
"Deleted bookmark title is null");
mAsserter.is(c.getString(c.getColumnIndex(BrowserContract.Bookmarks.URL)), null,
@ -724,6 +696,40 @@ public class testBrowserProvider extends ContentProviderTest {
"Deleted bookmark Favicon ID is null");
mAsserter.isnot(c.getString(c.getColumnIndex(BrowserContract.Bookmarks.GUID)), null,
"Deleted bookmark GUID is not null");
}
@Override
public void test() throws Exception {
// Test that unsynced bookmarks are not dropped from the database.
long id = insertOneBookmark();
int changed = mProvider.delete(BrowserContract.Bookmarks.CONTENT_URI,
BrowserContract.Bookmarks._ID + " = ?",
new String[] { String.valueOf(id) });
// Deletions also affect parents of folders, and so that must be accounted for.
mAsserter.is((changed == 2), true, "Inserted bookmark was deleted");
Cursor c = getBookmarkById(appendUriParam(BrowserContract.Bookmarks.CONTENT_URI, BrowserContract.PARAM_SHOW_DELETED, "1"), id);
mAsserter.is(c.moveToFirst(), true, "Unsynced deleted was only marked as deleted");
verifyMarkedAsDeleted(c);
c.close();
// Test that synced bookmarks are only marked as deleted.
id = insertOneBookmark("test-guid", true);
// Bookmark has been inserted from sync. Let's delete it again, and test that it has not
// been dropped from the database.
changed = mProvider.delete(BrowserContract.Bookmarks.CONTENT_URI,
BrowserContract.Bookmarks._ID + " = ?",
new String[] { String.valueOf(id) });
// Deletions also affect parents of folders, and so that must be accounted for.
mAsserter.is((changed == 2), true, "Inserted bookmark was deleted");
c = getBookmarkById(appendUriParam(BrowserContract.Bookmarks.CONTENT_URI, BrowserContract.PARAM_SHOW_DELETED, "1"), id);
mAsserter.is(c.moveToFirst(), true, "Deleted bookmark was only marked as deleted");
verifyMarkedAsDeleted(c);
c.close();
changed = mProvider.delete(appendUriParam(BrowserContract.Bookmarks.CONTENT_URI, BrowserContract.PARAM_IS_SYNC, "1"),