Add audible, muted to Tab, c.t.query, c.t.update, and c.t.onUpdated where relevant

chrome/test/data/extensions/api_test/tabs/basics/tabs_util.js

Index: chrome/test/data/extensions/api_test/tabs/basics/tabs_util.js
diff --git a/chrome/test/data/extensions/api_test/tabs/basics/tabs_util.js b/chrome/test/data/extensions/api_test/tabs/basics/tabs_util.js
index 757cd6b46539e019144ad5639f0ef65675a89c5e..6ac4cde0f80b7cededbb58b87bc960a8ab128de9 100644
--- a/chrome/test/data/extensions/api_test/tabs/basics/tabs_util.js
+++ b/chrome/test/data/extensions/api_test/tabs/basics/tabs_util.js
@@ -35,10 +35,10 @@ function createWindow(tabUrls, winOptions, callback) {
 // zero arguments.
 function waitForAllTabs(callback) {
   // Wait for all tabs to load.
-  function waitForTabs(){
+  function waitForTabs() {
     chrome.windows.getAll({"populate": true}, function(windows) {
       var ready = true;
-      for (var i in windows){
+      for (var i in windows) {
         for (var j in windows[i].tabs) {
           if (windows[i].tabs[j].status != "complete") {
             ready = false;
@@ -56,3 +56,53 @@ function waitForAllTabs(callback) {
   }
   waitForTabs();
 }
+
+// Check if query finds that queryAttrib is equal or not equal to value
+// (depending on expectedFound) and check that the attribute on the tab
+// object has the same value.  Calls callback afterward.
+var checkQuery = function(tabId, queryAttrib, value, expectedFound, callback) {

[not at google - send to devlin, 2015/05/26 23:09:48]

1. Declare with "function checkQuery" not "var checkQuery = function".

2. Avoid shortened names like "attrib".

3. A long list of arguments like this - particularly when it's mostly booleans -
is quite hard to read. Better is to use a single object argument (and document
the parameters) such that callers would use:

checkQuery({
  tabId: 42,
  attribute: ...,
  value: ...,
  expectFound: ...,
}, function() {
  ...
});

I don't actually understand the arguments, so that would help me review this as
well. Is there a more descriptive name than "checkQuery"?

[Jared Sohn, 2015/05/27 14:52:02]

Done
 
Done. (Also, renamed other fields and updated the description to hopefully make
it more clear how this works.)
Maybe verifyQueryForTab?  The purpose of the method is to create an object for a
c.t.query and check that the results for a particular tabid match/do not match
the expectedValue.

I have made some changes but am holding off on changing the method signature
until after we discuss this since this involves changing the caller code and I
would prefer to not have to do that too many times.  One aspect of this function
that might be considered confusing is the expectedFound parameter; the reasoning
behind it is that to know that a query works correctly I think you need to know
both that a match is found and that a nonmatch is not found.

An alternative suggestion: remove expectedValue and expectedFound as parameters
and return the results of the c.t.query for tabId (undefined if not found)
within the callback.  The calling code can then do an assert against the
expectedValue and handle the found/not found logic.  The method can be renamed
to something like queryForTab.

[not at google - send to devlin, 2015/05/28 23:04:42]

I *think* understand what you mean, and if that understanding is correct, then
what you suggest here sounds like the best option. It composes well.

+  var queryParams = {};
+  queryParams[queryAttrib] = value;
+  chrome.tabs.query(queryParams,
+    pass(function(tabs) {
+      var found = false;
+      tabs.forEach(function(tab) {
+        if (tab.id === tabId)
+          found = true;
+      });
+      assertEq(expectedFound, found);
+      if (callback !== null)
+        callback();
+    })
+  );
+};
+
+// Check onUpdated for a queryable attribute such as {muted, audible}
+// and then check that expected, changeInfo, the tab, and the query are
+// consistent.  Also checks that the expected values, changeInfo and tab
+// object for entries in otherAttribsDict are consistent.
+function onUpdatedExpect(attrib, expected, otherAttribsDict) {

[not at google - send to devlin, 2015/05/26 23:09:48]

1. I find the name confusing, perhaps because it doesn't read smoothly; the verb
"expect" I think should be at the start of the name, not the end.
"expectOnUpdated" almost looks better, but it's not really accurate. Perhaps
"assertAttributesOnUpdated"... don't like that much either.

2. See comment above about multiple arguments. "otherAttribsDict" doesn't mean
much to me.

[Jared Sohn, 2015/05/27 14:52:02]

How about expectAfterOnUpdated?
Arguments renamed and description updated to hopefully make this more clear. 
Like the previous function, I am holding off on making changes that affect the
method signature until after we discuss this more.

+  var onUpdatedCompleted = chrome.test.listenForever(
+    chrome.tabs.onUpdated,
+    function(tabId, changeInfo, tab) {
+      if (otherAttribsDict !== null) {
+        var otherAttribsKeys = Object.keys(otherAttribsDict);
+        otherAttribsKeys.forEach(function(attrib) {
+          if (typeof changeInfo[attrib] !== "undefined") {
+            assertEq(otherAttribsDict[attrib], changeInfo[attrib]);
+            assertEq(otherAttribsDict[attrib], tab[attrib]);
+          }
+        });
+      }
+      if (attrib in changeInfo) {
+        assertEq(expected, changeInfo[attrib]);
+        assertEq(expected, tab[attrib]);
+        checkQuery(tabId, attrib, expected, true, pass(function() {
+          checkQuery(tabId, attrib, !expected, false, pass(function() {
+            onUpdatedCompleted();
+          }));
+        }));
+      }
+    }
+  );
+}