Extension apis: windows.create({url:}) now supports relative paths.

tabs.update() & tabs.move() now return full Tab object.

BUG=17665

Review URL: http://codereview.chromium.org/203042

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

5 files changed, 116 insertions, 19 deletions

diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 01788ea..bc4cbe1 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -260,9 +260,14 @@ bool CreateWindowFunction::RunImpl() {
                                                   &url_input));
       url.reset(new GURL(url_input));
       if (!url->is_valid()) {
-        error_ = ExtensionErrorUtils::FormatErrorMessage(keys::kInvalidUrlError,
-            url_input);
-        return false;
+        // The path as passed in is not valid. Try converting to absolute path.
+        *url = GetExtension()->GetResourceURL(url_input);
+        if (!url->is_valid()) {
+          error_ = ExtensionErrorUtils::FormatErrorMessage(
+              keys::kInvalidUrlError,
+              url_input);
+          return false;
+        }
       }
     }
   }
@@ -598,6 +603,10 @@ bool UpdateTabFunction::RunImpl() {
     }
   }
 
+  if (has_callback())
+    result_.reset(ExtensionTabUtil::CreateTabValue(contents, tab_strip,
+        tab_index));
+
   return true;
 }
 
@@ -616,9 +625,10 @@ bool MoveTabFunction::RunImpl() {
 
   Browser* source_browser = NULL;
   TabStripModel* source_tab_strip = NULL;
+  TabContents* contents = NULL;
   int tab_index = -1;
-  if (!GetTabById(tab_id, profile(), &source_browser, &source_tab_strip, NULL,
-      &tab_index, &error_))
+  if (!GetTabById(tab_id, profile(), &source_browser, &source_tab_strip,
+      &contents, &tab_index, &error_))
     return false;
 
   if (update_props->HasKey(keys::kWindowIdKey)) {
@@ -635,7 +645,7 @@ bool MoveTabFunction::RunImpl() {
     if (ExtensionTabUtil::GetWindowId(target_browser) !=
         ExtensionTabUtil::GetWindowId(source_browser)) {
       TabStripModel* target_tab_strip = target_browser->tabstrip_model();
-      TabContents *contents = source_tab_strip->DetachTabContentsAt(tab_index);
+      contents = source_tab_strip->DetachTabContentsAt(tab_index);
       if (!contents) {
         error_ = ExtensionErrorUtils::FormatErrorMessage(
             keys::kTabNotFoundError, IntToString(tab_id));
@@ -650,6 +660,10 @@ bool MoveTabFunction::RunImpl() {
       target_tab_strip->InsertTabContentsAt(new_index, contents,
           false, true);
 
+      if (has_callback())
+        result_.reset(ExtensionTabUtil::CreateTabValue(contents,
+            target_tab_strip, new_index));
+
       return true;
     }
   }
@@ -662,7 +676,10 @@ bool MoveTabFunction::RunImpl() {
 
   if (new_index != tab_index)
     source_tab_strip->MoveTabContentsAt(tab_index, new_index, false);
-
+  
+  if (has_callback())
+    result_.reset(ExtensionTabUtil::CreateTabValue(contents, source_tab_strip,
+        new_index));
   return true;
 }
 
diff --git a/chrome/common/common_resources.grd b/chrome/common/common_resources.grd
index 6f3b735..5fc44f4 100644
--- a/chrome/common/common_resources.grd
+++ b/chrome/common/common_resources.grd
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!-- This comment is only here because changes to resources are not picked up
-without changes to the corresponding grd file. xji -->
+without changes to the corresponding grd file. rw -->
 <grit latest_public_release="0" current_release="1">
   <outputs>
     <output filename="grit/common_resources.h" type="rc_header">
diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json
index ffbae24..76fee6a 100755
--- a/chrome/common/extensions/api/extension_api.json
+++ b/chrome/common/extensions/api/extension_api.json
@@ -475,7 +475,18 @@
               "selected": {"type": "boolean", "optional": true}
             }
           },
-          {"type": "function", "name": "callback", "optional": true, "parameters": []}
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": [
+              {
+                "name": "tab",
+                "$ref": "Tab",
+                "description": "Details about the updated tab."
+              }
+            ]
+          }
         ]
       },
       {
@@ -501,7 +512,18 @@
               }
             }
           },
-          {"type": "function", "name": "callback", "optional": true, "parameters": []}
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": [
+              {
+                "name": "tab",
+                "$ref": "Tab",
+                "description": "Details about the moved tab."
+              }
+            ]
+          }
         ]
       },
       {
diff --git a/chrome/test/data/extensions/api_test/tabs/relative.html b/chrome/test/data/extensions/api_test/tabs/relative.html
new file mode 100755
index 0000000..5a9e6ee8
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/tabs/relative.html
@@ -0,0 +1,9 @@
+<html>
+<head>
+<script>
+window.onload = function() {
+  chrome.extension.getBackgroundPage().relativePageLoaded();
+}
+</script>
+</head>
+</html>
\ No newline at end of file
diff --git a/chrome/test/data/extensions/api_test/tabs/test.js b/chrome/test/data/extensions/api_test/tabs/test.js
index 0d95cf7..8c40e4e 100644
--- a/chrome/test/data/extensions/api_test/tabs/test.js
+++ b/chrome/test/data/extensions/api_test/tabs/test.js
@@ -16,6 +16,11 @@ var pass = chrome.test.callbackPass;
 var assertEq = chrome.test.assertEq;
 var assertTrue = chrome.test.assertTrue;
 
+// Called by relative.html during onload.
+function relativePageLoaded() {
+  chrome.test.succeed();
+}
+
 chrome.test.runTests([
   function getSelected() {
     chrome.tabs.getSelected(null, pass(function(tab) {
@@ -57,7 +62,7 @@ chrome.test.runTests([
         chrome.tabs.create({"windowId" : secondWindowId, "url" : "chrome://b"},
                            pass());
       }));
-   }));
+    }));
   },
 
   function getAllFirstWindow() {
@@ -97,8 +102,9 @@ chrome.test.runTests([
     chrome.tabs.get(testTabId, pass(function(tab) {
       assertEq("chrome://a/", tab.url);
       // Update url.
-      chrome.tabs.update(testTabId, {"url": "chrome://c"},
-                         pass(function(){
+      chrome.tabs.update(testTabId, {"url": "chrome://c/"},
+                         pass(function(tab){
+        chrome.test.assertEq("chrome://c/", tab.url);
         // Check url.
         chrome.tabs.get(testTabId, pass(function(tab) {
           assertEq("chrome://c/", tab.url);
@@ -113,15 +119,17 @@ chrome.test.runTests([
       assertEq(true, tabs[2].selected);
       // Select tab[1].
       chrome.tabs.update(tabs[1].id, {selected: true},
-                         pass(function(){
+                         pass(function(tab1){
         // Check update of tab[1].
+        chrome.test.assertEq(true, tab1.selected);     
         chrome.tabs.getAllInWindow(firstWindowId, pass(function(tabs) {
           assertEq(true, tabs[1].selected);
           assertEq(false, tabs[2].selected);
           // Select tab[2].
           chrome.tabs.update(tabs[2].id, {selected: true},
-                             pass(function(){
+                             pass(function(tab2){
             // Check update of tab[2].
+            chrome.test.assertEq(true, tab2.selected); 
             chrome.tabs.getAllInWindow(firstWindowId, pass(function(tabs) {
               assertEq(false, tabs[1].selected);
               assertEq(true, tabs[2].selected);
@@ -183,15 +191,19 @@ chrome.test.runTests([
   //  Window2: b,(newtab),d
   function moveTabs() {
     chrome.tabs.move(moveTabIds['b'], {"windowId": moveWindow2, "index": 0},
-                     pass(function() {
+                     pass(function(tabB) {
+        chrome.test.assertEq(0, tabB.index);
         chrome.tabs.move(moveTabIds['e'], {"index": 2},
-                         pass(function() {
+                         pass(function(tabE) {
+          chrome.test.assertEq(2, tabE.index);           
           chrome.tabs.move(moveTabIds['d'], {"windowId": moveWindow2,
-              "index": 2}, pass(function() {}));
+                           "index": 2}, pass(function(tabD) {
+            chrome.test.assertEq(2, tabD.index);     
+        }));
       }));
     }));
   },
-
+  
   // Check that the tab/window state is what we expect after doing moves.
   function moveTabsCheck() {
     chrome.tabs.getAllInWindow(moveWindow1, pass(function(tabs) {
@@ -309,6 +321,43 @@ chrome.test.runTests([
     });
 
     chrome.tabs.remove(moveTabIds['c'], pass());
+  },
+
+  function setupRelativeUrlTests() {
+    chrome.windows.create({}, pass(function(win) {
+      assertTrue(win.id > 0);
+      firstWindowId = win.id;
+
+      chrome.windows.getAll({}, pass(function(windows) {
+        for (var i = 0; i < windows.length; i++) {
+          if (windows[i].id != firstWindowId) {
+            chrome.windows.remove(windows[i].id, pass());
+          }
+        }
+      }));
+    }));
+  },
+  
+  // The subsequent three tests all load relative.html, which calls
+  // this page's relativePageLoad(), which ends the test.
+  function relativeUrlTabsCreate() {
+    chrome.tabs.create({windowId: firstWindowId, url: 'relative.html'},
+      pass(function(tab){
+        testTabId = tab.id;
+      }
+    ));
+  },
+
+  function relativeUrlTabsUpdate() {
+    chrome.tabs.update(testTabId, {url: "chrome://a/"}, function(tab) {
+      chrome.test.assertEq("chrome://a/", tab.url);
+      chrome.tabs.update(tab.id, {url: "relative.html"}, function(tab) {  
+      });
+    });
+  },
+  
+  function relativeUrlWindowsCreate() {
+    chrome.windows.create({url: "relative.html"});
   }
 
   // TODO(asargent) We still need to add tests for the following: