Issue 2034 - Respect chrome.webRequest.MAX_HANDLER_BEHAVIOR_CHANGED_CALLS_PER_10_MINUTES

chrome/ext/background.js

Index: chrome/ext/background.js
===================================================================
--- a/chrome/ext/background.js
+++ b/chrome/ext/background.js
@@ -326,9 +326,35 @@
     return (framesOfTabs[tabId] || {})[frameId];
   };
 
+  var handlerBehaviorChangedQuota = chrome.webRequest.MAX_HANDLER_BEHAVIOR_CHANGED_CALLS_PER_10_MINUTES;
+
+  function handlerBehaviorChanged()

[Wladimir Palant, 2015/04/09 18:45:02]

That's a confusing name. We have three things all called handlerBehaviorChanged:
Chrome's API function, our API function and this private function. While the
former two have similar meaning, the latter doesn't.

Maybe rename it into something like propagateChange()?

[Sebastian Noack, 2015/04/10 06:45:26]

Good point. I went for propagateHandlerBehaviorChange().

+  {
+    // Make sure to not call handlerBehaviorChanged() more often than allowed
+    // by chrome.webRequest.MAX_HANDLER_BEHAVIOR_CHANGED_CALLS_PER_10_MINUTES.
+    // Otherwise Chrome notifies the user that this extension is causing issues.
+    if (handlerBehaviorChangedQuota > 0)
+    {
+      chrome.webNavigation.onBeforeNavigate.removeListener(handlerBehaviorChanged);
+      chrome.webRequest.handlerBehaviorChanged();
+
+      handlerBehaviorChangedQuota--;
+      setTimeout(function() { handlerBehaviorChangedQuota++; }, 600000);

[Wladimir Palant, 2015/04/09 18:45:02]

This will "recover" the quota way too slowly. I guess the timeout should be
600000/chrome.webRequest.MAX_HANDLER_BEHAVIOR_CHANGED_CALLS_PER_10_MINUTES?

[Sebastian Noack, 2015/04/10 06:45:26]

I think it is correct. This makes sure to not call handlerBehaviorChanged() more
ofthen than MAX_HANDLER_BEHAVIOR_CHANGED_CALLS_PER_10_MINUTES within 10 minutes.
This would recover the quota way to quickly (every 30 seconds), potentially
calling handlerBehaviorChanged() way more frequently than
MAX_HANDLER_BEHAVIOR_CHANGED_CALLS_PER_10_MINUTES within 10 minutes.

[Wladimir Palant, 2015/04/10 06:58:14]

You are right, the current approach is correct.

+    }
+  }
+
   ext.webRequest = {
     onBeforeRequest: new ext._EventTarget(),
-    handlerBehaviorChanged: chrome.webRequest.handlerBehaviorChanged
+    handlerBehaviorChanged: function()
+    {
+      // Defer handlerBehaviorChanged() until navigation occurs.
+      // There wouldn't be any visible effect when calling it earlier,
+      // but it's an expensive operation and that way we avoid to call
+      // it multiple times, if multiple filters are added/removed.
+      var onBeforeNavigate = chrome.webNavigation.onBeforeNavigate;
+      if (!onBeforeNavigate.hasListener(handlerBehaviorChanged))
+        onBeforeNavigate.addListener(handlerBehaviorChanged);

[Wladimir Palant, 2015/04/09 18:45:02]

My initial reaction was: "cannot be that the same listener will be added twice."
Well, it can...

[Sebastian Noack, 2015/04/10 06:45:26]

Yep, it can. ;)

+    }
   };
 
   // Since Chrome 38 requests of type 'object' (e.g. requests