Fixed ExtensionApiTest.Bookmarks and enabled it again.

Also fixed a little nit in the chrome.bookmarks.create() call, which would return the wrong error code when given an invalid URL.

BUG=18979,89762
TEST=ExtensionApiTest.Bookmarks in the browser_tests is green


Review URL: https://chromiumcodereview.appspot.com/10831012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148556 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/bookmarks/bookmark_extension_api.cc

@@ -504,7 +504,7 @@ bool CreateBookmarkFunction::RunImpl() {
   std::string url_string;
   json->GetString(keys::kUrlKey, &url_string);  // Optional.
   GURL url(url_string);
-  if (!url.is_empty() && !url.is_valid()) {
+  if (!url_string.empty() && !url.is_valid()) {
     error_ = keys::kInvalidUrlError;
     return false;
   }

chrome/browser/bookmarks/bookmark_extension_apitest.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/browser/extensions/extension_apitest.h"
 
-// Flaky test, http://crbug.com/89762.
-IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DISABLED_Bookmarks) {
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Bookmarks) {
   ASSERT_TRUE(RunExtensionTest("bookmarks")) << message_;
 }

chrome/test/data/extensions/api_test/bookmarks/test.js

@@ -67,7 +67,7 @@ function compareTrees(left, right, verbose) {
     return true;
   }
   if (left == null || right == null)
-    return left + " !+ " + right;
+    return left + " != " + right;
   if (left.length != right.length)
     return "count mismatch: " + left.length + " != " + right.length;
   for (var i = 0; i < left.length; i++) {
@@ -177,6 +177,7 @@ chrome.test.runTests([
       // Make sure the parentId defaults to the Other Bookmarks folder.
       chrome.test.assertEq(expected[0].children[1].id, created.parentId);
       chrome.test.assertTrue(compareNode(node, created));
+      expected[0].children[1].children.push(node);
     });
     chrome.bookmarks.create(node, pass(function(results) {
       node.id = results.id;  // since we couldn't know this going in
@@ -276,12 +277,11 @@ chrome.test.runTests([
                           pass(function(results) {
       chrome.test.assertEq(results.parentId, other.id);
       folder.parentId = results.parentId;
+      folder.index = results.index;
 
       var folder2 = expected[0].children[0].children.pop();
-      folder2.parentId = other.parentId;
-      folder2.index = 0;
+      chrome.test.assertEq(folder2.id, folder.id);
       expected[0].children[1].children.push(folder2);
-
       verifyTreeIsExpected(pass());
     }));
   },
@@ -361,9 +361,9 @@ chrome.test.runTests([
       // Update expected to match.
       // We removed node1, which means that the index of the other two nodes
       // changes as well.
-      expected[0].children[1].children[0].children.shift();
-      expected[0].children[1].children[0].children[0].index = 0;
-      expected[0].children[1].children[0].children[1].index = 1;
+      expected[0].children[1].children[1].children.shift();
+      expected[0].children[1].children[1].children[0].index = 0;
+      expected[0].children[1].children[1].children[1].index = 1;
 
       verifyTreeIsExpected(pass());
     }));
@@ -371,7 +371,7 @@ chrome.test.runTests([
 
   function removeTree() {
     var parentId = node2.parentId;
-    var folder = expected[0].children[1].children[0];
+    var folder = expected[0].children[1].children[1];
     chrome.test.listenOnce(chrome.bookmarks.onRemoved,
                            function(id, removeInfo) {
       chrome.test.assertEq(id, folder.id);
@@ -380,8 +380,7 @@ chrome.test.runTests([
     });
     chrome.bookmarks.removeTree(parentId, pass(function(){
       // Update expected to match.
-      expected[0].children[1].children.shift();
-
+      expected[0].children[1].children.pop();
       verifyTreeIsExpected(pass());
     }));
   },
@@ -436,9 +435,8 @@ chrome.test.runTests([
       chrome.bookmarks.getChildren(id, pass(function(children) {
         children.forEach(function(child, i) {
           chrome.bookmarks.removeTree(child.id, pass(function() {}));
-
           // When we have removed the last child we can continue.
-          if (i == children.length - 1)
+          if (id == "2" && i == children.length - 1)
             afterRemove();
         });
       }));