Adding Discarded property support for onUpdate function.

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

Index: chrome/browser/extensions/api/tabs/tabs_event_router.cc
diff --git a/chrome/browser/extensions/api/tabs/tabs_event_router.cc b/chrome/browser/extensions/api/tabs/tabs_event_router.cc
index 80c495c96736c1de6aa06d668eb52bdbe8adad82..84c5e15c106c2070ce36011c6129cec25280880a 100644
--- a/chrome/browser/extensions/api/tabs/tabs_event_router.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_event_router.cc
@@ -11,10 +11,12 @@
 
 #include "base/memory/ptr_util.h"
 #include "base/values.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/api/tabs/tabs_constants.h"
 #include "chrome/browser/extensions/api/tabs/tabs_windows_api.h"
 #include "chrome/browser/extensions/api/tabs/windows_event_router.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
+#include "chrome/browser/memory/tab_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -140,9 +142,14 @@ TabsEventRouter::TabsEventRouter(Profile* profile)
 
   browser_tab_strip_tracker_.Init(
       BrowserTabStripTracker::InitWith::ALL_BROWERS);
+
+  if (g_browser_process->GetTabManager())

[Devlin, 2016/07/29 16:01:09]

Document when this is null.

[Anderson Silva, 2016/07/29 19:05:59]

It shouldn't be null anymore as we included a tab_manager for
TestingBrowserProcess.

+    g_browser_process->GetTabManager()->AddObserver(this);

[Devlin, 2016/07/29 16:01:09]

Use a ScopedObserver.

[Anderson Silva, 2016/07/29 19:05:59]

Done.

 }
 
 TabsEventRouter::~TabsEventRouter() {
+  if (g_browser_process->GetTabManager())
+    g_browser_process->GetTabManager()->RemoveObserver(this);

[Devlin, 2016/07/29 16:01:09]

Is the TabManager guaranteed to outlive the TabsEventRouter (in the case that it
exists at all)?  Similarly, if it *will* ever exist, is it guaranteed to exist
at this object's construction time?

[Anderson Silva, 2016/07/29 19:05:59]

TabManager is created at the first time GetTabManager() is invoked, so it'll
always exist. As it lives in g_browser_process, it should also outlive
TabsEventRouter.
However, as I added ScopedObserver now, this shouldn't be an issue anymore.

[Devlin, 2016/07/29 20:17:33]

Not strictly true.  A ScopedObserver just automatically calls RemoveObserver()
on any sources that are still registered when it's destructed; it doesn't
prevent a UAF if the owning object outlives the object it's observing.  (But
given the TabManager is only destructed when the browser process is, we should
be okay.)

 }
 
 bool TabsEventRouter::ShouldTrackBrowser(Browser* browser) {
@@ -552,4 +559,16 @@ void TabsEventRouter::OnFaviconUpdated(
   }
 }
 
+void TabsEventRouter::OnDiscardedStateChange(WebContents* contents,
+                                             bool is_discarded) {
+  TabStripModel* tab_strip = nullptr;
+  int tab_index = -1;
+
+  if (ExtensionTabUtil::GetTabStripModel(contents, &tab_strip, &tab_index)) {

[Devlin, 2016/07/29 16:01:09]

Why do we use this method here instead of e.g. GetTabId?

[Anderson Silva, 2016/07/29 19:05:59]

I see this call just as a check that we can find a the tab strip for this
web_contents. We won't really use any of the information, we'll just make sure
there's a tab strip associated with this contents before sending events.

[Devlin, 2016/07/29 20:17:33]

Do we strictly need a tab strip associated?  Or can we just check that there's
an ID associated?

+    std::set<std::string> changed_property_names;
+    changed_property_names.insert(tabs_constants::kDiscardedKey);
+    DispatchTabUpdatedEvent(contents, std::move(changed_property_names));
+  }
+}
+
 }  // namespace extensions