[Android] JNI bridges for querying top level bookmark folders.

chrome/browser/android/bookmarks/bookmarks_bridge.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/android/bookmarks/bookmarks_bridge.h"
 
 #include "base/android/jni_string.h"
 #include "base/containers/stack_container.h"
+#include "base/i18n/string_compare.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/bookmarks/chrome_bookmark_client_factory.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_android.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/common/pref_names.h"
 #include "components/bookmarks/browser/bookmark_model.h"
@@ skipping 14 matching lines @@
 using content::BrowserThread;
 
 namespace {
 
 class BookmarkNodeCreationTimeCompareFunctor {
  public:
   bool operator()(const BookmarkNode* lhs, const BookmarkNode* rhs) {
     return lhs->date_added().ToJavaTime() > rhs->date_added().ToJavaTime();
   }
 };
+
+class BookmarkNodeTitleCompareFunctor {

[newt (away), 2014/09/03 01:49:51]

simplify this name? maybe BookmarkTitleComparer

[Kibeom Kim (inactive), 2014/09/03 06:29:44]

Done.

+ public:
+  explicit BookmarkNodeTitleCompareFunctor(const icu::Collator* collator)
+      : collator_(collator) {}
+
+  bool operator()(const BookmarkNode* lhs, const BookmarkNode* rhs) {
+    if (collator_) {
+      return base::i18n::CompareString16WithCollator(
+          collator_, lhs->GetTitle(), rhs->GetTitle()) == UCOL_LESS;
+    } else {
+      return lhs->GetTitle() < rhs->GetTitle();
+    }
+  }
+
+private:
+  const icu::Collator* collator_;
+};
+
+// Note: If you use this class to get BookmarkNodeTitleCompareFunctor, then
+// this factory must outlive the functor instance because this factory is
+// responsible for icu::Collator lifetime that the functor depends on.
+class BookmarkNodeTitleCompareFunctorFactory {

[newt (away), 2014/09/03 00:51:50]

What's the point of making this a separate class? Seems like
BookmarkNodeTitleCompareFunctorFactory could be merged into
BookmarkNodeTitleCompareFunctor, and this avoids the lifetime issues you
mentioned in the comment above.

[Kibeom Kim (inactive), 2014/09/03 00:56:00]

I thought so.. and tried that first, it turned out that std::sort requries
Compare functor to be copy-constructible. If not, it gives a bunch of compiler
errors :) .

I think the most right way to get around this problem is using
std::reference_wrapper on the functor object, something like:

std::sort(x.begin(), x.end(), std::cref(BookmarkNodeTitleCompareFunctor()));


but since c++ not available, this was an alternative (thanks to trchen@ for the
suggestion).

[newt (away), 2014/09/03 01:49:51]

I'd add a comment explaining this, since it's far from obvious :)

[Kibeom Kim (inactive), 2014/09/03 06:29:44]

Done.

+ public:
+  BookmarkNodeTitleCompareFunctorFactory() {
+    UErrorCode error = U_ZERO_ERROR;
+    collator_.reset(icu::Collator::createInstance(error));
+    if (U_FAILURE(error)) {
+      collator_.reset(NULL);
+    }
+  }
+
+  BookmarkNodeTitleCompareFunctor GetFunctor() {
+    return BookmarkNodeTitleCompareFunctor(collator_.get());
+  }
+
+ private:
+  scoped_ptr<icu::Collator> collator_;
+};
+
 }  // namespace
 
 BookmarksBridge::BookmarksBridge(JNIEnv* env,
                                  jobject obj,
                                  jobject j_profile)
     : weak_java_ref_(env, obj),
       bookmark_model_(NULL),
       client_(NULL),
       partner_bookmarks_shim_(NULL) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ skipping 59 matching lines @@
                                                              jobject obj,
                                                              jlong id,
                                                              jint type) {
   DCHECK(IsLoaded());
   return CreateJavaBookmark(GetNodeByID(id, type));
 }
 
 void BookmarksBridge::GetPermanentNodeIDs(JNIEnv* env,
                                           jobject obj,
                                           jobject j_result_obj) {
+  // TODO(kkimlabs): Remove this function.
   DCHECK(IsLoaded());
 
   base::StackVector<const BookmarkNode*, 8> permanent_nodes;
 
   // Save all the permanent nodes.
   const BookmarkNode* root_node = bookmark_model_->root_node();
   permanent_nodes->push_back(root_node);
   for (int i = 0; i < root_node->child_count(); ++i) {
     permanent_nodes->push_back(root_node->GetChild(i));
   }
   permanent_nodes->push_back(
       partner_bookmarks_shim_->GetPartnerBookmarksRoot());
 
   // Write the permanent nodes to |j_result_obj|.
   for (base::StackVector<const BookmarkNode*, 8>::ContainerType::const_iterator
            it = permanent_nodes->begin();
        it != permanent_nodes->end();
        ++it) {
     if (*it != NULL) {
       Java_BookmarksBridge_addToBookmarkIdList(
           env, j_result_obj, (*it)->id(), GetBookmarkType(*it));
     }
   }
 }
 
+void BookmarksBridge::GetTopLevelFolderParentIDs(JNIEnv* env,
+                                                 jobject obj,
+                                                 jobject j_result_obj) {
+  Java_BookmarksBridge_addToBookmarkIdList(
+      env, j_result_obj, bookmark_model_->root_node()->id(),
+      GetBookmarkType(bookmark_model_->root_node()));
+  Java_BookmarksBridge_addToBookmarkIdList(
+      env, j_result_obj, bookmark_model_->mobile_node()->id(),
+      GetBookmarkType(bookmark_model_->mobile_node()));
+  Java_BookmarksBridge_addToBookmarkIdList(
+        env, j_result_obj, bookmark_model_->other_node()->id(),
+        GetBookmarkType(bookmark_model_->other_node()));
+}
+
+void BookmarksBridge::GetTopLevelFolderIDs(JNIEnv* env,
+                                           jobject obj,
+                                           jboolean get_special,
+                                           jboolean get_normal,
+                                           jobject j_result_obj) {
+  DCHECK(IsLoaded());
+  std::vector<const BookmarkNode*> top_level_folders;
+
+  if (get_special) {
+    if (client_->managed_node() &&
+        client_->managed_node()->child_count() > 0) {
+      top_level_folders.push_back(client_->managed_node());
+    }
+    // TODO(kkimlabs): add partner bookmark root node, if available.
+  }
+  std::size_t special_count = top_level_folders.size();
+
+  if (get_normal) {
+    DCHECK_EQ(bookmark_model_->root_node()->child_count(), 4);
+
+    top_level_folders.push_back(bookmark_model_->bookmark_bar_node());
+
+    const BookmarkNode* mobile_node = bookmark_model_->mobile_node();
+    for (int i = 0; i < mobile_node->child_count(); ++i) {
+      top_level_folders.push_back(mobile_node->GetChild(i));
+    }
+
+    const BookmarkNode* other_node = bookmark_model_->other_node();
+    for (int i = 0; i < other_node->child_count(); ++i) {
+      top_level_folders.push_back(other_node->GetChild(i));
+    }
+
+    std::stable_sort(top_level_folders.begin() + special_count,
+                     top_level_folders.end(),
+                     BookmarkNodeTitleCompareFunctorFactory().GetFunctor());

[newt (away), 2014/09/03 01:49:51]

It seems you're not obeying the lifetime rules you laid out above. IIUC the
BookmarkNodeTitleCompareFunctorFactory could be destructed before
std::stable_sort is called. It would be more likely to work across compilers if
you assigned BookmarkNodeTitleCompareFunctorFactory to a local variable before
calling std::stable_sort.

[Kibeom Kim (inactive), 2014/09/03 06:29:44]

I think it's safe because BookmarkNodeTitleCompareFunctorFactory is guaranteed
to be destructed after std::stable_sort by c++ spec.
http://stackoverflow.com/questions/12323049/when-do-temporary-parameter-value...

+  }
+
+  for (std::vector<const BookmarkNode*>::const_iterator it =
+      top_level_folders.begin(); it != top_level_folders.end(); ++it) {
+    Java_BookmarksBridge_addToBookmarkIdList(env,
+                                             j_result_obj,
+                                             (*it)->id(),
+                                             GetBookmarkType(*it));
+  }
+}
+
 void BookmarksBridge::GetChildIDs(JNIEnv* env,
                                   jobject obj,
                                   jlong id,
                                   jint type,
                                   jboolean get_folders,
                                   jboolean get_bookmarks,
                                   jobject j_result_obj) {
   DCHECK(IsLoaded());
 
   const BookmarkNode* parent = GetNodeByID(id, type);
@@ skipping 495 matching lines @@
   BookmarkModelChanged();
 }
 
 void BookmarksBridge::PartnerShimLoaded(PartnerBookmarksShim* shim) {
   NotifyIfDoneLoading();
 }
 
 void BookmarksBridge::ShimBeingDeleted(PartnerBookmarksShim* shim) {
   partner_bookmarks_shim_ = NULL;
 }