tabId support to chrome.extensions.getViews()

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

Index: chrome/test/data/extensions/api_test/get_views/test.js
diff --git a/chrome/test/data/extensions/api_test/get_views/test.js b/chrome/test/data/extensions/api_test/get_views/test.js
index 765c4c0fb42605d6414f40e033c306b5609b3342..f6c0c6fc84c108bb6f49005838defe940f4cc8fc 100644
--- a/chrome/test/data/extensions/api_test/get_views/test.js
+++ b/chrome/test/data/extensions/api_test/get_views/test.js
@@ -11,20 +11,47 @@ const assertTrue = chrome.test.assertTrue;
 // We need to remember the popupWindowId to be able to find it later.
 var popupWindowId = 0;
 
+// Updated and used later to check getViews() tabId tag for the options tab.
+var optionsTabId = 0;
+
 // This function is called by the popup during the test run.
 function popupCallback() {
-  // We have now added an popup so the total count goes up one.
+  // We have now added a popup so the total count goes up one.
   assertEq(2, chrome.extension.getViews().length);
-  assertEq(1, chrome.extension.getViews({"windowId": popupWindowId}).length);
-
-  chrome.tabs.create({"url": chrome.extension.getURL("options.html")});
+  assertEq(1, chrome.extension.getViews({windowId: popupWindowId}).length);
+  chrome.tabs.create({url: chrome.extension.getURL("options.html")},

[Devlin, 2016/07/13 16:22:59]

nitty nit: argument parameters should line up with one another or all be on the
same line, so this should be something like:

chrome.tabs.create({url: chrome.extension.getURL("options.html")},
                   function(tab) {
  optionsTabId = tab.id;
});

[catmullings, 2016/07/13 22:07:06]

Done.

+    function(tab) {
+      optionsTabId = tab.id;
+  });
 }
 
 function optionsPageCallback() {
   assertEq(3, chrome.extension.getViews().length);
-  assertEq(1, chrome.extension.getViews({"windowId": popupWindowId}).length);
-  assertEq(2, chrome.extension.getViews(
-      {"type": "tab", "windowId": window.id}).length);
+  assertEq(1, chrome.extension.getViews({windowId: popupWindowId}).length);
+  assertEq(2,
+      chrome.extension.getViews({type: "tab", windowId: window.id}).length);

[Devlin, 2016/07/13 16:22:59]

ditto re parameter line-up:
assertEq(
    2, chrome.extension.getViews({type: "tab", windowId: window.id}).length);

[catmullings, 2016/07/13 22:07:06]

Done.

+
+  chrome.tabs.query({windowId: window.id}, function(tabs) {
+    // Assert 3 to account for new tab page, web popup, & new tab options.html

[Devlin, 2016/07/13 16:22:59]

need to end this comment in a '.'

[catmullings, 2016/07/13 22:07:06]

Done.

+    assertEq(3, tabs.length);
+
+    // Check that only 2 tabs belong to the extension

[Devlin, 2016/07/13 16:22:59]

ditto, though here this comment just describes the exactly what the line
immediately below does, so it should probably be rephrased.  Something like:
// getViews() only returns views that run in the extension process, so
// this should only include the popup and options.html pages.

[catmullings, 2016/07/13 22:07:06]

Yeah, I did address this before, but it didn't get committed, for some reason.
I had written the following:

// Two tabs represent the extension's window popup and options.html

But I will use your comment since it gives more context.

[catmullings, 2016/07/13 22:07:06]

Done.

+    assertEq(2, chrome.extension.getViews({type:"tab"}).length);
+
+    for (var i = 0; i < tabs.length; i++) {
+      if (tabs[i].windowId == popupWindowId) {
+        assertEq(1, chrome.extension.getViews({tabId: tabs[i].id}).length);
+         // Test tabId tag with other parameters

[Devlin, 2016/07/13 16:22:59]

end in a '.'

[catmullings, 2016/07/13 22:07:06]

Done.

+        assertEq(1, chrome.extension.getViews({windowId: popupWindowId,
+                                               tabId: tabs[i].id}).length);
+        assertEq(1, chrome.extension.getViews({type: "tab",
+                                               tabId: tabs[i].id}).length);
+      } else if (tabs[i].id == optionsTabId) {
+        assertEq(1, chrome.extension.getViews({tabId: tabs[i].id}).length);
+      }
+    }
+  });
+
   chrome.test.notifyPass();
 }
 
@@ -32,15 +59,18 @@ var tests = [
   function getViews() {
     assertTrue(typeof(chrome.extension.getBackgroundPage()) != "undefined");
     assertEq(1, chrome.extension.getViews().length);
-    assertEq(0, chrome.extension.getViews({"type": "tab"}).length);
-    assertEq(0, chrome.extension.getViews({"type": "popup"}).length);
+    assertEq(0, chrome.extension.getViews({type: "tab"}).length);
+    assertEq(0, chrome.extension.getViews({type: "popup"}).length);
 
     chrome.windows.getAll({populate: true}, function(windows) {
       assertEq(1, windows.length);
 
       // Create a popup window.
-      chrome.windows.create({"url": chrome.extension.getURL("popup.html"),
-                             "type": "popup"}, function(window) {
+
+      // TODO (catmullings): Fix potential race condition when/if
+      // popupCallback() is called before popupWindowId is set below
+      chrome.windows.create({url: chrome.extension.getURL("popup.html"),
+                             type: "popup"}, function(window) {
         assertTrue(window.id > 0);
         popupWindowId = window.id;
         // The popup will call back to us through popupCallback (above).