[WIP] Bug 1232439 (972193) - Only include desktop folder if a sync has occurred. draft
authorTom Klein <twointofive@gmail.com>
Wed, 21 Dec 2016 01:03:07 -0600
changeset 453952 28d57b8b4ec6f8972181294c8ad7738dbfbbca83
parent 453951 a007bfd879cc0579c1d3966f19abf977ddd13ffe
child 453953 9f72271a7109c0be70f338df61b9bcbe2b2294da
child 488488 b22dc0a401d333431cfdb7304d172329f0d6198c
push id39776
push userbmo:twointofive@gmail.com
push dateMon, 26 Dec 2016 21:15:51 +0000
bugs1232439, 972193
milestone53.0a1
[WIP] Bug 1232439 (972193) - Only include desktop folder if a sync has occurred. With some minor cleanup edits in the folder tree fetching code. MozReview-Commit-ID: 1ZPswpYV2tG
mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
@@ -27,17 +27,17 @@ import android.support.v4.content.Cursor
 
 /**
  * Interface for interactions with all databases. If you want an instance
  * that implements this, you should go through GeckoProfile. E.g.,
  * <code>BrowserDB.from(context)</code>.
  */
 public abstract class BrowserDB {
     public class BookmarkFolderTree {
-        // Using a TreeMap allows walking the folders in alphabetical order
+        // Using a TreeMap allows walking the folders in alphabetical order.
         // In most cases the Key will correspond to the folder's name, however this is not true
         // in all cases. We use artificial keys for the Mobile/Desktop folder to guarantee
         // ordering within the root.
         public TreeMap<String, BookmarkFolderTree> children = new TreeMap<>();
 
         public final int id;
         public final int parent;
         public final String name;
--- a/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
@@ -1999,27 +1999,16 @@ public class LocalBrowserDB extends Brow
 
     @Override
     public void blockActivityStreamSite(ContentResolver cr, String url) {
         final ContentValues values = new ContentValues();
         values.put(ActivityStreamBlocklist.URL, url);
         cr.insert(mActivityStreamBlockedUriWithProfile, values);
     }
 
-    private static void debugDumpFolder(BookmarkFolderTree folder, String prefix) {
-        if (folder == null) {
-            return;
-        }
-
-        Log.d("***FOLDERS", prefix + folder.name);
-        for (BookmarkFolderTree f : folder.children.values()) {
-            debugDumpFolder(f, prefix + "*");
-        }
-    }
-
     public BookmarkFolderTree getBookmarkFolders(ContentResolver cr) {
         final Cursor c = cr.query(mBookmarksUriWithProfile,
                 new String[] {
                         Bookmarks._ID,
                         Bookmarks.PARENT,
                         Bookmarks.GUID,
                         Bookmarks.TITLE
                 },
@@ -2034,19 +2023,23 @@ public class LocalBrowserDB extends Brow
                 null);
 
         if (c == null) {
             return null;
         }
 
         final HashMap<Integer, BookmarkFolderTree> nodes = new HashMap<>();
 
-        final String desktopFolderName = "Desktop";
-        final BookmarkFolderTree fakeDesktop = new BookmarkFolderTree(Bookmarks.FAKE_DESKTOP_FOLDER_ID, Bookmarks.FIXED_ROOT_ID, desktopFolderName);
-        nodes.put(Bookmarks.FAKE_DESKTOP_FOLDER_ID, fakeDesktop);
+        final boolean showFakeDesktopFolder = desktopBookmarksExist(cr);
+        if (showFakeDesktopFolder) {
+            final String desktopFolderName = "Desktop";
+            final BookmarkFolderTree fakeDesktop =
+                    new BookmarkFolderTree(Bookmarks.FAKE_DESKTOP_FOLDER_ID, Bookmarks.FIXED_ROOT_ID, desktopFolderName);
+            nodes.put(Bookmarks.FAKE_DESKTOP_FOLDER_ID, fakeDesktop);
+        }
 
         final int idIndex = c.getColumnIndexOrThrow(Bookmarks._ID);
         final int parentIndex = c.getColumnIndexOrThrow(Bookmarks.PARENT);
         final int nameIndex = c.getColumnIndexOrThrow(Bookmarks.TITLE);
         final int guidIndex = c.getColumnIndexOrThrow(Bookmarks.GUID);
 
         try {
             if (!c.moveToFirst()) {
@@ -2058,27 +2051,32 @@ public class LocalBrowserDB extends Brow
                 final int parentId;
 
                 switch (guid) {
                     // By default these are subfolders of places, however we want to make them
                     // subfolders of Desktop Bookmarks.
                     case Bookmarks.TOOLBAR_FOLDER_GUID:
                     case Bookmarks.UNFILED_FOLDER_GUID:
                     case Bookmarks.MENU_FOLDER_GUID:
-                        parentId = Bookmarks.FAKE_DESKTOP_FOLDER_ID;
+                        if (showFakeDesktopFolder) {
+                            parentId = Bookmarks.FAKE_DESKTOP_FOLDER_ID;
+                        } else {
+                            // Don't include these folders.
+                            continue;
+                        }
                         break;
 
                     default:
                         parentId = c.getInt(parentIndex);
                 }
 
                 final String folderName = c.getString(nameIndex);
                 final int id = c.getInt(idIndex);
 
-                BookmarkFolderTree folder = new BookmarkFolderTree(id, parentId, folderName);
+                final BookmarkFolderTree folder = new BookmarkFolderTree(id, parentId, folderName);
 
                 // We want to ignore any folders that aren't either a subfolder of one of the 3 desktop folders,
                 // or a subfolder of mobile. I.e. we want to discard all folders with places as their parent
                 // (which includes the pinned folder, and "All Bookmarks"), except for those 3 desktop folders (handled
                 // above), and mobile bookmarks and places itself (which has itself as its parent) -
                 // handled here.
                 if (parentId != Bookmarks.FIXED_ROOT_ID ||
                         guid.equals(Bookmarks.MOBILE_FOLDER_GUID) ||
@@ -2086,18 +2084,18 @@ public class LocalBrowserDB extends Brow
                     nodes.put(folder.id, folder);
                 }
             } while (c.moveToNext());
         } finally {
             c.close();
         }
 
         // Build the tree!
-        for (BookmarkFolderTree folder : nodes.values()) {
-            BookmarkFolderTree parent = nodes.get(folder.parent);
+        for (final BookmarkFolderTree folder : nodes.values()) {
+            final BookmarkFolderTree parent = nodes.get(folder.parent);
 
             if (parent == null || parent == folder) {
                 // We need to avoid parenting:
                 // (1) the root / places folder: it is its own parent, attaching it would create a loop
                 // (2) orphaned folders - this can happen if sync has problems
                 continue;
             }
 
@@ -2112,19 +2110,17 @@ public class LocalBrowserDB extends Brow
                     // BookmarkFolderTree seems wasteful.
                     parent.children.put("A", folder);
                 }
             } else {
                 parent.children.put(folder.name, folder);
             }
         }
 
-        final BookmarkFolderTree root = nodes.get(0);
+        final BookmarkFolderTree root = nodes.get(Bookmarks.FIXED_ROOT_ID);
 
         if (root == null) {
             throw new IllegalStateException("root / places bookmarks folder must exist");
         }
 
-        debugDumpFolder(root, "*");
-
         return root;
     }
 }