From f951acb3916621dc24432482f9e10f7b62afa0b8 Mon Sep 17 00:00:00 2001 From: Robert Strong Date: Thu, 24 Feb 2011 03:20:02 -0800 Subject: [PATCH] Bug 631092 - Large number of blocklist requestors making multiple requests per day affecting the ping for metrics. r=dtownsend, a=approval2.0 --- toolkit/mozapps/extensions/AddonManager.jsm | 4 +- .../mozapps/extensions/nsBlocklistService.js | 63 +++++++++++-------- .../test/xpcshell/test_bug620837.js | 56 ++++++++++++++--- 3 files changed, 88 insertions(+), 35 deletions(-) diff --git a/toolkit/mozapps/extensions/AddonManager.jsm b/toolkit/mozapps/extensions/AddonManager.jsm index f952c8039f80..84c90eb32bfb 100644 --- a/toolkit/mozapps/extensions/AddonManager.jsm +++ b/toolkit/mozapps/extensions/AddonManager.jsm @@ -41,7 +41,7 @@ const Cc = Components.classes; const Ci = Components.interfaces; const Cr = Components.results; -const PREF_BLOCKLIST_PINGCOUNT = "extensions.blocklist.pingCount"; +const PREF_BLOCKLIST_PINGCOUNTVERSION = "extensions.blocklist.pingCountVersion"; const PREF_EM_UPDATE_ENABLED = "extensions.update.enabled"; const PREF_EM_LAST_APP_VERSION = "extensions.lastAppVersion"; const PREF_EM_AUTOUPDATE_DEFAULT = "extensions.update.autoUpdateDefault"; @@ -241,7 +241,7 @@ var AddonManagerInternal = { LOG("Application has been upgraded"); Services.prefs.setCharPref(PREF_EM_LAST_APP_VERSION, Services.appinfo.version); - Services.prefs.setIntPref(PREF_BLOCKLIST_PINGCOUNT, + Services.prefs.setIntPref(PREF_BLOCKLIST_PINGCOUNTVERSION, (appChanged === undefined ? 0 : -1)); } diff --git a/toolkit/mozapps/extensions/nsBlocklistService.js b/toolkit/mozapps/extensions/nsBlocklistService.js index 09749f53142f..cec88e5d4a91 100644 --- a/toolkit/mozapps/extensions/nsBlocklistService.js +++ b/toolkit/mozapps/extensions/nsBlocklistService.js @@ -58,8 +58,8 @@ const PREF_BLOCKLIST_URL = "extensions.blocklist.url"; const PREF_BLOCKLIST_ENABLED = "extensions.blocklist.enabled"; const PREF_BLOCKLIST_INTERVAL = "extensions.blocklist.interval"; const PREF_BLOCKLIST_LEVEL = "extensions.blocklist.level"; -const PREF_BLOCKLIST_PINGCOUNT = "extensions.blocklist.pingCount"; -const PREF_BLOCKLIST_TOTALPINGCOUNT = "extensions.blocklist.totalPingCount"; +const PREF_BLOCKLIST_PINGCOUNTTOTAL = "extensions.blocklist.pingCountTotal"; +const PREF_BLOCKLIST_PINGCOUNTVERSION = "extensions.blocklist.pingCountVersion"; const PREF_PLUGINS_NOTIFYUSER = "plugins.update.notifyUser"; const PREF_GENERAL_USERAGENT_LOCALE = "general.useragent.locale"; const PREF_PARTNER_BRANCH = "app.partner."; @@ -429,24 +429,31 @@ Blocklist.prototype = { return; } - var pingCount = getPref("getIntPref", PREF_BLOCKLIST_PINGCOUNT, 0); - var totalPingCount = getPref("getIntPref", PREF_BLOCKLIST_TOTALPINGCOUNT, 1); - var daysSinceLastPing; - if (pingCount < 1) { - daysSinceLastPing = pingCount == 0 ? "new" : "reset"; - pingCount = 1; + var pingCountVersion = getPref("getIntPref", PREF_BLOCKLIST_PINGCOUNTVERSION, 0); + var pingCountTotal = getPref("getIntPref", PREF_BLOCKLIST_PINGCOUNTTOTAL, 1); + var daysSinceLastPing = 0; + if (pingCountVersion < 1 || pingCountTotal < 1) { + daysSinceLastPing = pingCountVersion == 0 ? "new" : "reset"; + if (pingCountVersion < 1) + pingCountVersion = 1; + if (pingCountTotal < 1) + pingCountTotal = 1; } else { // Seconds in one day is used because nsIUpdateTimerManager stores the // last update time in seconds. let secondsInDay = 60 * 60 * 24; let lastUpdateTime = getPref("getIntPref", PREF_BLOCKLIST_LASTUPDATETIME, 0); - if (lastUpdateTime != 0) { + if (lastUpdateTime == 0) { + daysSinceLastPing = "invalid"; + } + else { let now = Math.round(Date.now() / 1000); daysSinceLastPing = Math.floor((now - lastUpdateTime) / secondsInDay); } - else { - daysSinceLastPing = "invalid"; + + if (daysSinceLastPing == 0 || daysSinceLastPing == "invalid") { + pingCountVersion = pingCountTotal = "invalid"; } } @@ -464,29 +471,33 @@ Blocklist.prototype = { getDistributionPrefValue(PREF_APP_DISTRIBUTION)); dsURI = dsURI.replace(/%DISTRIBUTION_VERSION%/g, getDistributionPrefValue(PREF_APP_DISTRIBUTION_VERSION)); - dsURI = dsURI.replace(/%PING_COUNT%/g, pingCount); - dsURI = dsURI.replace(/%TOTAL_PING_COUNT%/g, totalPingCount); + dsURI = dsURI.replace(/%PING_COUNT%/g, pingCountVersion); + dsURI = dsURI.replace(/%TOTAL_PING_COUNT%/g, pingCountTotal); dsURI = dsURI.replace(/%DAYS_SINCE_LAST_PING%/g, daysSinceLastPing); dsURI = dsURI.replace(/\+/g, "%2B"); // Under normal operations it will take around 5,883,516 years before the - // preferences used to store pingCount and totalPingCount will rollover + // preferences used to store pingCountVersion and pingCountTotal will rollover // so this code doesn't bother trying to do the "right thing" here. - pingCount++; - if (pingCount > 2147483647) { - // Rollover to -1 if the value is greater than what is support by an - // integer preference. The -1 indicates that the counter has been reset. - pingCount = -1; + if (pingCountVersion != "invalid") { + pingCountVersion++; + if (pingCountVersion > 2147483647) { + // Rollover to -1 if the value is greater than what is support by an + // integer preference. The -1 indicates that the counter has been reset. + pingCountVersion = -1; + } + gPref.setIntPref(PREF_BLOCKLIST_PINGCOUNTVERSION, pingCountVersion); } - gPref.setIntPref(PREF_BLOCKLIST_PINGCOUNT, pingCount); - totalPingCount++; - if (totalPingCount > 2147483647) { - // Rollover to 1 if the value is greater than what is support by an - // integer preference. - totalPingCount = 1; + if (pingCountTotal != "invalid") { + pingCountTotal++; + if (pingCountTotal > 2147483647) { + // Rollover to 1 if the value is greater than what is support by an + // integer preference. + pingCountTotal = -1; + } + gPref.setIntPref(PREF_BLOCKLIST_PINGCOUNTTOTAL, pingCountTotal); } - gPref.setIntPref(PREF_BLOCKLIST_TOTALPINGCOUNT, totalPingCount); // Verify that the URI is valid try { diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_bug620837.js b/toolkit/mozapps/extensions/test/xpcshell/test_bug620837.js index 5ad71de473c8..35953a922bb5 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/test_bug620837.js +++ b/toolkit/mozapps/extensions/test/xpcshell/test_bug620837.js @@ -4,8 +4,9 @@ do_load_httpd_js(); -const PREF_BLOCKLIST_LASTUPDATETIME = "app.update.lastUpdateTime.blocklist-background-update-timer"; -const PREF_BLOCKLIST_PINGCOUNT = "extensions.blocklist.pingCount"; +const PREF_BLOCKLIST_LASTUPDATETIME = "app.update.lastUpdateTime.blocklist-background-update-timer"; +const PREF_BLOCKLIST_PINGCOUNTTOTAL = "extensions.blocklist.pingCountTotal"; +const PREF_BLOCKLIST_PINGCOUNTVERSION = "extensions.blocklist.pingCountVersion"; const SECONDS_IN_DAY = 60 * 60 * 24; @@ -50,7 +51,7 @@ function test1() { function test2() { gNextTest = test3; - gExpectedQueryString = "2&2&invalid"; + gExpectedQueryString = "invalid&invalid&invalid"; notify_blocklist(); } @@ -58,24 +59,65 @@ function test3() { Services.prefs.setIntPref(PREF_BLOCKLIST_LASTUPDATETIME, (getNowInSeconds() - SECONDS_IN_DAY)); gNextTest = test4; - gExpectedQueryString = "3&3&1"; + gExpectedQueryString = "2&2&1"; notify_blocklist(); } function test4() { - Services.prefs.setIntPref(PREF_BLOCKLIST_PINGCOUNT, -1); + Services.prefs.setIntPref(PREF_BLOCKLIST_PINGCOUNTVERSION, -1); Services.prefs.setIntPref(PREF_BLOCKLIST_LASTUPDATETIME, (getNowInSeconds() - (SECONDS_IN_DAY * 2))); gNextTest = test5; - gExpectedQueryString = "1&4&reset"; + gExpectedQueryString = "1&3&reset"; notify_blocklist(); } function test5() { + Services.prefs.setIntPref(PREF_BLOCKLIST_LASTUPDATETIME, getNowInSeconds()); + gNextTest = test6; + gExpectedQueryString = "invalid&invalid&0"; + notify_blocklist(); +} + +function test6() { Services.prefs.setIntPref(PREF_BLOCKLIST_LASTUPDATETIME, (getNowInSeconds() - (SECONDS_IN_DAY * 3))); + gNextTest = test7; + gExpectedQueryString = "2&4&3"; + notify_blocklist(); +} + +function test7() { + Services.prefs.setIntPref(PREF_BLOCKLIST_PINGCOUNTVERSION, 2147483647); + Services.prefs.setIntPref(PREF_BLOCKLIST_LASTUPDATETIME, + (getNowInSeconds() - (SECONDS_IN_DAY * 4))); + gNextTest = test8; + gExpectedQueryString = "2147483647&5&4"; + notify_blocklist(); +} + +function test8() { + Services.prefs.setIntPref(PREF_BLOCKLIST_LASTUPDATETIME, + (getNowInSeconds() - (SECONDS_IN_DAY * 5))); + gNextTest = test9; + gExpectedQueryString = "1&6&reset"; + notify_blocklist(); +} + +function test9() { + Services.prefs.setIntPref(PREF_BLOCKLIST_PINGCOUNTTOTAL, 2147483647); + Services.prefs.setIntPref(PREF_BLOCKLIST_LASTUPDATETIME, + (getNowInSeconds() - (SECONDS_IN_DAY * 6))); + gNextTest = test10; + gExpectedQueryString = "2&2147483647&6"; + notify_blocklist(); +} + +function test10() { + Services.prefs.setIntPref(PREF_BLOCKLIST_LASTUPDATETIME, + (getNowInSeconds() - (SECONDS_IN_DAY * 7))); gNextTest = finish; - gExpectedQueryString = "2&5&3"; + gExpectedQueryString = "3&1&reset"; notify_blocklist(); }