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

chrome/browser/extensions/api/tabs/tabs_event_router.h

Index: chrome/browser/extensions/api/tabs/tabs_event_router.h
diff --git a/chrome/browser/extensions/api/tabs/tabs_event_router.h b/chrome/browser/extensions/api/tabs/tabs_event_router.h
index 73ee2c95945800530d26e0e89ebcf410b8c8dc4a..a58d91ac10518b0722406966d3200f048fc5967d 100644
--- a/chrome/browser/extensions/api/tabs/tabs_event_router.h
+++ b/chrome/browser/extensions/api/tabs/tabs_event_router.h
@@ -16,6 +16,7 @@
 #include "components/ui/zoom/zoom_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "extensions/browser/event_router.h"
+#include "extensions/common/value_builder.h"
 
 namespace content {
 class WebContents;
@@ -31,6 +32,9 @@ class TabsEventRouter : public TabStripModelObserver,
                         public chrome::BrowserListObserver,
                         public content::NotificationObserver,
                         public ui_zoom::ZoomObserver {
+ private:
+  class TabEntry;

[not at google - send to devlin, 2015/03/16 18:11:56]

Declare this above TabUpdated. Odd having a private: section up the top.

Though really the whole class definition of TabEntry should be the first thing
in the private: block, making the forward declaration unnecessary, but whatever.
Don't do that, unnecessary churn :-)

[not at google - send to devlin, 2015/03/19 21:23:35]

Not addressed?

+
  public:
   explicit TabsEventRouter(Profile* profile);
   ~TabsEventRouter() override;
@@ -82,7 +86,7 @@ class TabsEventRouter : public TabStripModelObserver,
 
   // Internal processing of tab updated events. Is called by both TabChangedAt
   // and Observe/NAV_ENTRY_COMMITTED.
-  void TabUpdated(content::WebContents* contents, bool did_navigate);
+  void TabUpdated(TabEntry* entry, DictionaryBuilder* changed_properties);
 
   // Triggers a tab updated event if the favicon URL changes.
   void FaviconUrlUpdated(content::WebContents* contents);
@@ -129,6 +133,7 @@ class TabsEventRouter : public TabStripModelObserver,
   //  - distinguish between tab creation and tab insertion
   //  - not send tab-detached after tab-removed
   //  - reduce the "noise" of TabChangedAt() when sending events to extensions
+  //  - remember last muted and audible states to know if there was a change
   class TabEntry {
    public:
     // Create a new tab entry whose initial state is TAB_COMPLETE.  This
@@ -136,19 +141,29 @@ class TabsEventRouter : public TabStripModelObserver,
     // std::map<> by value.
     TabEntry();
 
+    // Allow creating a TabEntry via a webcontents.  Used when moving
+    // tabs between windows.
+    explicit TabEntry(content::WebContents* contents);
+
     // Update the load state of the tab based on its WebContents.  Returns true
     // if the state changed, false otherwise.  Whether the state has changed or

[not at google - send to devlin, 2015/03/16 18:11:55]

Comment needs to be rewritten, this doesn't return true/false

     // not is used to determine if events needs to be sent to extensions during
     // processing of TabChangedAt(). This method will "hold" a state-change
     // to "loading", until the DidNavigate() method which should always follow
     // it. Returns NULL if no updates should be sent.
-    base::DictionaryValue* UpdateLoadState(
-        const content::WebContents* contents);
+    DictionaryBuilder* UpdateLoadState();

[not at google - send to devlin, 2015/03/16 18:11:55]

Better to pass a scoped_ptr<DictionaryBuilder> here (and from DidNavigate) to
indicate that ownership is passed out of this function.

Unfortunately this is a little bit awkward with the current design of
DictionaryValue - the whole point of DictionaryBuilder is so that you can chain
together constructions, like I mentioned in the .cc file. You might as well just
use DictionaryValue otherwise. However. the .Set() methods return a
DictionaryValue& not DictionaryValue*.

Proposal: add a .Release() method to DictionaryBuilder which returns a
scoped_ptr to itself. You can then write the code I mentioned in the .cc file.

*However*, all that said - perhaps we should just be returning DictionaryValue
(well, scoped_ptr<DictionaryValue>). That seems more correct to me and avoids
this awkwardness altogether.

 
     // Indicates that a tab load has resulted in a navigation and the
     // destination url is available for inspection. Returns NULL if no updates
     // should be sent.
-    base::DictionaryValue* DidNavigate(const content::WebContents* contents);
+    DictionaryBuilder* DidNavigate();
+
+    // Update the audible and muted states and return whether they were changed
+    bool SetAudible(bool new_val);
+    bool SetMuted(bool new_val);
+
+    // TODO(jaredsohn):  make it private or add public accessor

[not at google - send to devlin, 2015/03/16 18:11:55]

Please do this now - public web_contents() accessor SGTM.

+    content::WebContents* contents_;
 
    private:
     // Whether we are waiting to fire the 'complete' status change. This will
@@ -157,6 +172,10 @@ class TabsEventRouter : public TabStripModelObserver,
     // loading state subsequently, but we will ignore those changes.
     bool complete_waiting_on_load_;
 
+    // Previous audible and muted states
+    bool was_audible_;
+    bool was_muted_;
+
     GURL url_;
   };