Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 6615790883176448: Issue 2034 - Respect chrome.webRequest.MAX_HANDLER_BEHAVIOR_CHANGED_CALLS_PER_10_MINUTES (Closed)

Created:
Feb. 23, 2015, 5:18 p.m. by Sebastian Noack
Modified:
April 13, 2015, 10:30 a.m.
Reviewers:
kzar, Wladimir Palant
Visibility:
Public.

Description

Issue 2034 - Respect chrome.webRequest.MAX_HANDLER_BEHAVIOR_CHANGED_CALLS_PER_10_MINUTES

Patch Set 1 #

Total comments: 4

Patch Set 2 : Rebased and deferred handlerBehaviorChanged() until navigation occurs #

Total comments: 7

Patch Set 3 : Renamed function to propagateHandlerBehaviorChange #

Total comments: 2

Patch Set 4 : Renamed overlooked variable name #

Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -14 lines) Patch
M chrome/ext/background.js View 1 2 3 1 chunk +27 lines, -1 line 0 comments Download
M webrequest.js View 1 2 chunks +1 line, -13 lines 0 comments Download

Messages

Total messages: 10
Sebastian Noack
Feb. 23, 2015, 5:19 p.m. (2015-02-23 17:19:46 UTC) #1
Wladimir Palant
http://codereview.adblockplus.org/6615790883176448/diff/5629499534213120/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6615790883176448/diff/5629499534213120/chrome/ext/background.js#newcode354 chrome/ext/background.js:354: } Is this really a good approach? I think ...
Feb. 23, 2015, 6:25 p.m. (2015-02-23 18:25:26 UTC) #2
Sebastian Noack
http://codereview.adblockplus.org/6615790883176448/diff/5629499534213120/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6615790883176448/diff/5629499534213120/chrome/ext/background.js#newcode354 chrome/ext/background.js:354: } We call handlerBehaviorChanged() only if it did. That ...
Feb. 23, 2015, 7:12 p.m. (2015-02-23 19:12:08 UTC) #3
Wladimir Palant
http://codereview.adblockplus.org/6615790883176448/diff/5629499534213120/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6615790883176448/diff/5629499534213120/chrome/ext/background.js#newcode354 chrome/ext/background.js:354: } If users are changing filters frequently (more often ...
Feb. 23, 2015, 7:22 p.m. (2015-02-23 19:22:42 UTC) #4
Sebastian Noack
http://codereview.adblockplus.org/6615790883176448/diff/5629499534213120/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6615790883176448/diff/5629499534213120/chrome/ext/background.js#newcode354 chrome/ext/background.js:354: } On 2015/02/23 19:22:42, Wladimir Palant wrote: > If ...
April 9, 2015, 7:04 a.m. (2015-04-09 07:04:55 UTC) #5
Wladimir Palant
http://codereview.adblockplus.org/6615790883176448/diff/5634387206995968/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6615790883176448/diff/5634387206995968/chrome/ext/background.js#newcode331 chrome/ext/background.js:331: function handlerBehaviorChanged() That's a confusing name. We have three ...
April 9, 2015, 6:45 p.m. (2015-04-09 18:45:01 UTC) #6
Sebastian Noack
http://codereview.adblockplus.org/6615790883176448/diff/5634387206995968/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6615790883176448/diff/5634387206995968/chrome/ext/background.js#newcode331 chrome/ext/background.js:331: function handlerBehaviorChanged() On 2015/04/09 18:45:02, Wladimir Palant wrote: > ...
April 10, 2015, 6:45 a.m. (2015-04-10 06:45:25 UTC) #7
Wladimir Palant
http://codereview.adblockplus.org/6615790883176448/diff/5634387206995968/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6615790883176448/diff/5634387206995968/chrome/ext/background.js#newcode342 chrome/ext/background.js:342: setTimeout(function() { handlerBehaviorChangedQuota++; }, 600000); You are right, the ...
April 10, 2015, 6:58 a.m. (2015-04-10 06:58:13 UTC) #8
Sebastian Noack
http://codereview.adblockplus.org/6615790883176448/diff/5718998062727168/chrome/ext/background.js File chrome/ext/background.js (right): http://codereview.adblockplus.org/6615790883176448/diff/5718998062727168/chrome/ext/background.js#newcode338 chrome/ext/background.js:338: chrome.webNavigation.onBeforeNavigate.removeListener(handlerBehaviorChanged); On 2015/04/10 06:58:14, Wladimir Palant wrote: > Feel ...
April 10, 2015, 7:03 a.m. (2015-04-10 07:03:21 UTC) #9
Wladimir Palant
April 10, 2015, 7:08 a.m. (2015-04-10 07:08:33 UTC) #10
LGTM

Powered by Google App Engine
This is Rietveld