Add Instrumental Test for BookmarksBridge

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"
@@ skipping 128 matching lines @@
 
 static bool IsEditBookmarksEnabled() {
   return ProfileManager::GetLastUsedProfile()->GetPrefs()->GetBoolean(
       bookmarks::prefs::kEditBookmarksEnabled);
 }
 
 static jboolean IsEditBookmarksEnabled(JNIEnv* env, jclass clazz) {
   return IsEditBookmarksEnabled();
 }
 
+void BookmarksBridge::LoadEmptyPartnerBookmarkShimForTesting(JNIEnv* env,
+                                                             jobject obj) {
+  if (partner_bookmarks_shim_->IsLoaded()) return;

[Ted C, 2014/09/16 02:03:52]

in c++, the return goes on the next line.

It is fine to have no parens in the c++ case for single line
conditional/statement combinations.

[Ian Wen, 2014/09/16 20:31:34]

Done.

+  partner_bookmarks_shim_->SetPartnerBookmarksRoot(
+      new BookmarkPermanentNode(0));
+  DCHECK(partner_bookmarks_shim_->IsLoaded());
+}
+
 ScopedJavaLocalRef<jobject> BookmarksBridge::GetBookmarkByID(JNIEnv* env,
                                                              jobject obj,
                                                              jlong id,
                                                              jint type) {
   DCHECK(IsLoaded());
   return CreateJavaBookmark(GetNodeByID(id, type));
 }
 
 bool BookmarksBridge::IsDoingExtensiveChanges(JNIEnv* env, jobject obj) {
   return bookmark_model_->IsDoingExtensiveChanges();
@@ skipping 190 matching lines @@
              bookmarkList.rbegin();
          it != bookmarkList.rend();
          ++it) {
       stk.push(std::make_pair(*it, depth + 1));
     }
   }
 }
 
 ScopedJavaLocalRef<jobject> BookmarksBridge::GetMobileFolderId(JNIEnv* env,
                                                                jobject obj) {
   const BookmarkNode* mobileNode = bookmark_model_->mobile_node();

[Ted C, 2014/09/16 02:03:52]

since you're here, can you change this to mobile_node like the other comments
below?

[Ian Wen, 2014/09/16 20:31:34]

Done.

   ScopedJavaLocalRef<jobject> folder_id_obj =
       Java_BookmarksBridge_createBookmarkId(
           env, mobileNode->id(), GetBookmarkType(mobileNode));
   return folder_id_obj;
 }
 
+ScopedJavaLocalRef<jobject> BookmarksBridge::GetOtherFolderId(JNIEnv* env,
+                                                              jobject obj) {
+  const BookmarkNode* otherNode = bookmark_model_->other_node();

[Ted C, 2014/09/16 02:03:52]

s/otherNode/other_node

[Ian Wen, 2014/09/16 20:31:34]

Done.

+  ScopedJavaLocalRef<jobject> folder_id_obj =
+      Java_BookmarksBridge_createBookmarkId(
+          env, otherNode->id(), GetBookmarkType(otherNode));
+  return folder_id_obj;
+}
+
+ScopedJavaLocalRef<jobject> BookmarksBridge::GetDesktopFolderId(JNIEnv* env,
+                                                                jobject obj) {
+  const BookmarkNode* desktopNode = bookmark_model_->bookmark_bar_node();

[Ted C, 2014/09/16 02:03:52]

s/desktopNode/desktop_node

[Ian Wen, 2014/09/16 20:31:34]

Done.

+  ScopedJavaLocalRef<jobject> folder_id_obj =
+      Java_BookmarksBridge_createBookmarkId(
+          env, desktopNode->id(), GetBookmarkType(desktopNode));
+  return folder_id_obj;
+}
+
 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 167 matching lines @@
   const BookmarkNode* node = folder;
   while (node) {
     ExtractBookmarkNodeInformation(node, j_result_obj);
     node = GetParentNode(node);
   }
 
   Java_BookmarksCallback_onBookmarksFolderHierarchyAvailable(
       env, j_callback_obj, j_folder_id_obj, j_result_obj);
 }
 
+ScopedJavaLocalRef<jobject> BookmarksBridge::AddFolder(JNIEnv* env,
+                                                       jobject obj,
+                                                       jobject j_parent_id_obj,
+                                                       jint index,
+                                                       jstring j_title) {
+  DCHECK(IsLoaded());
+  long bookmark_id = JavaBookmarkIdGetId(env, j_parent_id_obj);
+  int type = JavaBookmarkIdGetType(env, j_parent_id_obj);
+  const BookmarkNode* parent = GetNodeByID(bookmark_id, type);
+  if (type != BookmarkType::NORMAL) {
+    NOTREACHED();
+    return ScopedJavaLocalRef<jobject>();
+  }
+  const BookmarkNode* newNode = bookmark_model_->AddFolder(

[Ted C, 2014/09/16 02:03:52]

s/newNode/new_node

[Ted C, 2014/09/16 02:03:53]

I would add a blank line above this line as well.

Also, this can return null if passed an invalid ID.  Then it will just crash on
the next line as you call ->id() on it.

[Ian Wen, 2014/09/16 20:31:34]

Done.

+      parent, index, base::android::ConvertJavaStringToUTF16(env, j_title));
+  ScopedJavaLocalRef<jobject> new_java_obj =
+      Java_BookmarksBridge_createBookmarkId(
+          env, newNode->id(), GetBookmarkType(newNode));
+  return new_java_obj;
+}
+
 void BookmarksBridge::DeleteBookmark(JNIEnv* env,
                                      jobject obj,
                                      jobject j_bookmark_id_obj) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(IsLoaded());
 
   long bookmark_id = JavaBookmarkIdGetId(env, j_bookmark_id_obj);
   int type = JavaBookmarkIdGetType(env, j_bookmark_id_obj);
   const BookmarkNode* node = GetNodeByID(bookmark_id, type);
   if (!IsEditable(node)) {
@@ skipping 23 matching lines @@
   if (!IsEditable(node)) {
     NOTREACHED();
     return;
   }
   bookmark_id = JavaBookmarkIdGetId(env, j_parent_id_obj);
   type = JavaBookmarkIdGetType(env, j_parent_id_obj);
   const BookmarkNode* new_parent_node = GetNodeByID(bookmark_id, type);
   bookmark_model_->Move(node, new_parent_node, index);
 }
 
+ScopedJavaLocalRef<jobject> BookmarksBridge::AddBookmark(
+    JNIEnv* env,
+    jobject obj,
+    jobject j_parent_id_obj,
+    jint index,
+    jstring j_title,
+    jstring j_url) {
+  DCHECK(IsLoaded());
+  long bookmark_id = JavaBookmarkIdGetId(env, j_parent_id_obj);
+  int type = JavaBookmarkIdGetType(env, j_parent_id_obj);
+  const BookmarkNode* parent = GetNodeByID(bookmark_id, type);
+  if (type != BookmarkType::NORMAL) {
+    NOTREACHED();
+    return ScopedJavaLocalRef<jobject>();
+  }
+  const BookmarkNode* newNode = bookmark_model_->AddURL(

[Ted C, 2014/09/16 02:03:52]

s/newNode/new_node

Blank line here too

[Ian Wen, 2014/09/16 20:31:34]

Done.

+      parent,
+      index,
+      base::android::ConvertJavaStringToUTF16(env, j_title),
+      GURL(base::android::ConvertJavaStringToUTF16(env, j_url)));
+  ScopedJavaLocalRef<jobject> new_java_obj =
+      Java_BookmarksBridge_createBookmarkId(
+          env, newNode->id(), GetBookmarkType(newNode));
+  return new_java_obj;
+}
+
 ScopedJavaLocalRef<jobject> BookmarksBridge::CreateJavaBookmark(
     const BookmarkNode* node) {
   JNIEnv* env = AttachCurrentThread();
 
   const BookmarkNode* parent = GetParentNode(node);
   int64 parent_id = parent ? parent->id() : -1;
 
   std::string url;
   if (node->is_url())
     url = node->url().spec();
@@ skipping 265 matching lines @@
   BookmarkModelChanged();
 }
 
 void BookmarksBridge::PartnerShimLoaded(PartnerBookmarksShim* shim) {
   NotifyIfDoneLoading();
 }
 
 void BookmarksBridge::ShimBeingDeleted(PartnerBookmarksShim* shim) {
   partner_bookmarks_shim_ = NULL;
 }