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

 // Copyright (c) 2011 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.
 
 var pass = chrome.test.callbackPass;
 var fail = chrome.test.callbackFail;
 var assertEq = chrome.test.assertEq;
 var assertFalse = chrome.test.assertFalse;
 var assertTrue = chrome.test.assertTrue;
 
@@ skipping 17 matching lines @@
     callback(win.id, newTabIds);
   });
 }
 
 // Waits until all tabs (yes, in every window) have status "complete".
 // This is useful to prevent test overlap when testing tab events.
 // |callback| should look like function() {...}.  Note that |callback| expects
 // 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;
             break;
           }
         }
         if (!ready)
           break;
       }
       if (ready)
         callback();
       else
         window.setTimeout(waitForTabs, 30);
     });
   }
   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();
+          }));
+        }));
+      }
+    }
+  );
+}