Issue 7257 - Throttle badge updates

lib/stats.js

Index: lib/stats.js
===================================================================
--- a/lib/stats.js
+++ b/lib/stats.js
@@ -20,79 +20,141 @@
 "use strict";
 
 const {Prefs} = require("./prefs");
 const {BlockingFilter} = require("../adblockpluscore/lib/filterClasses");
 const {filterNotifier} = require("../adblockpluscore/lib/filterNotifier");
 const {port} = require("./messaging");
 
 const badgeColor = "#646464";
+
+// 4 fps.
+const badgeRefreshRate = 4;

[Manish Jethani, 2019/02/03 13:20:21]

I found this to be a reasonable refresh rate, YMMV.

+
 let blockedPerPage = new ext.PageMap();
 
 let getBlockedPerPage =
 /**
  * Gets the number of requests blocked on the given page.
  *
  * @param  {Page} page
  * @return {Number}
  */
 exports.getBlockedPerPage = page => blockedPerPage.get(page) || 0;
 
-function updateBadge(page, blockedCount)
+// The ID of the focused window. If all windows lose focus, this is the ID of
+// the last focused window.
+let focusedWindowId = -1;
+
+// The ID of the active tab in the focused window. This can be unknown (-1) on
+// startup and (briefly) when the focus shifts to a different window.
+let activeTabId = -1;
+
+let badgeUpdateScheduler = new ext.Scheduler();
+
+function updateBadgeNow()
 {
-  if (Prefs.show_statsinicon)
+  if (activeTabId == -1)
+    return;
+
+  let page = new ext.Page({id: activeTabId});
+  let blockedCount = blockedPerPage.get(page);
+
+  page.browserAction.setBadge(blockedCount && {
+    color: badgeColor,
+    number: blockedCount
+  });
+}
+
+function updateBadge(tabId = -1)
+{
+  if (!badgeUpdateScheduler.scheduled &&

[Manish Jethani, 2019/02/03 13:20:21]

The conditions are ordered from most likely to least likely.

+      (tabId == activeTabId || tabId == -1) && activeTabId != -1 &&
+      Prefs.show_statsinicon)
   {
-    page.browserAction.setBadge(blockedCount && {
-      color: badgeColor,
-      number: blockedCount
-    });
+    // Schedule an update.
+    badgeUpdateScheduler.schedule(1000 / badgeRefreshRate, updateBadgeNow);
   }
 }
 
+function findAndUpdateActiveTab()
+{
+  browser.tabs.query({active: true, lastFocusedWindow: true}, ([tab]) =>
+  {
+    if (tab && activeTabId != tab.id)
+    {
+      activeTabId = tab.id;
+      focusedWindowId = tab.windowId;
+
+      updateBadge();
+    }
+  });
+}
+
 // Once nagivation for the tab has been committed to (e.g. it's no longer
 // being prerendered) we clear its badge, or if some requests were already
 // blocked beforehand we display those on the badge now.
 browser.webNavigation.onCommitted.addListener(details =>
 {
   if (details.frameId == 0)
-  {
-    let page = new ext.Page({id: details.tabId});
-    let blocked = blockedPerPage.get(page);
-
-    updateBadge(page, blocked);
-  }
+    updateBadge(details.tabId);
 });
 
 filterNotifier.on("filter.hitCount", (filter, newValue, oldValue, tabIds) =>
 {
   if (!(filter instanceof BlockingFilter))
     return;
 
   for (let tabId of tabIds)
   {
     let page = new ext.Page({id: tabId});
     let blocked = blockedPerPage.get(page) || 0;
 
     blockedPerPage.set(page, ++blocked);
-    updateBadge(page, blocked);
+    updateBadge(tabId);
   }
 
   Prefs.blocked_total++;
 });
 
 Prefs.on("show_statsinicon", () =>
 {
   browser.tabs.query({}, tabs =>
   {
     for (let tab of tabs)
     {
       let page = new ext.Page(tab);
 
       if (Prefs.show_statsinicon)
-        updateBadge(page, blockedPerPage.get(page));
+        updateBadge(tab.id);
       else
         page.browserAction.setBadge(null);
     }
   });
 });
 
 port.on("stats.getBlockedPerPage",
         message => getBlockedPerPage(new ext.Page(message.tab)));
+
+browser.tabs.onActivated.addListener(tab =>
+{
+  if (tab.windowId == focusedWindowId && activeTabId != tab.tabId)

[Manish Jethani, 2019/02/03 13:20:21]

We care about only the focused window, not other windows.

[Sebastian Noack, 2019/02/03 23:18:03]

But it's possible that several windows showing the browser action are visible
(e.g. I quite often split my screen with one browser window on each side).

As for only updating the active tab, does that add much performance benefits?
Does updating the badge for an inactive tab even cause anything to be drawn?

[Manish Jethani, 2019/02/04 05:33:04]

Yes, that's possible, and in that case only the number in the focused window
will get updated. How common is it for people to (1) split their screens and (2)
care about every window's toolbar showing the most up-to-date number of ads
blocked? I doubt that there are too many people who would care about this.

The moment you focus on the window of course the number gets updated
immediately.
Yes, it does make a significant difference, see the description in the Trac
ticket [1]

[1]: https://issues.adblockplus.org/ticket/7257
I hope not, especially after Trac #7253, but the API call and the processing
that comes with it is also significant. It starts to show in performance
benchmarks. It is also common for users to click "Open Link in New Tab" (Command
+ click on macOS) while they are reading an article; I often open 4-5 links from
an article in a new tab (also often from Google search results) while I continue
to read. All that processing can be avoided.

[Sebastian Noack, 2019/02/04 06:32:04]

I think my case is at least not any less common than having another browser
window in the background.

[Manish Jethani, 2019/02/04 06:57:17]

Yes, but those are two different things. Windows in the background are almost
entirely obscured. You can't even see them, much less pay attention to the
incrementing counter. 

+  {
+    activeTabId = tab.tabId;
+    updateBadge();
+  }
+});
+
+if ("windows" in browser)
+{
+  browser.windows.onFocusChanged.addListener(windowId =>
+  {
+    if (windowId == browser.windows.WINDOW_ID_NONE)

[Manish Jethani, 2019/02/03 13:20:21]

Make sure focusedWindowId always points to a valid window. If no window is
focused, it's the last focused window.

+      return;
+
+    focusedWindowId = windowId;
+    activeTabId = -1;
+
+    findAndUpdateActiveTab();
+  });
+}
+
+findAndUpdateActiveTab();