Allow extension function call quota to be un-throttled.

For example, previously if chrome.storage.sync.set was called more than 1000
times in an hour it would be throttled forever (until Chrome restart). Now
that quota is restored every hour.

BUG=406406
R=zea@chromium.org

Review URL: https://codereview.chromium.org/704453002

Cr-Commit-Position: refs/heads/master@{#302716}

extensions/browser/quota_service.cc

@@ -57,11 +57,6 @@ std::string QuotaService::Assess(const std::string& extension_id,
   if (heuristics.empty())
     return std::string();  // No heuristic implies no limit.
 
-  ViolationErrorMap::iterator violation_error =
-      violation_errors_.find(extension_id);
-  if (violation_error != violation_errors_.end())
-    return violation_error->second;  // Repeat offender.
-
   QuotaLimitHeuristic* failed_heuristic = NULL;
   for (QuotaLimitHeuristics::iterator heuristic = heuristics.begin();
        heuristic != heuristics.end();
@@ -78,10 +73,6 @@ std::string QuotaService::Assess(const std::string& extension_id,
 
   std::string error = failed_heuristic->GetError();
   DCHECK_GT(error.length(), 0u);
-
-  PurgeFunctionHeuristicsMap(&functions);
-  function_heuristics_.erase(extension_id);
-  violation_errors_[extension_id] = error;
   return error;
 }
 

extensions/browser/quota_service.h

@@ -64,12 +64,9 @@ class QuotaService : public base::NonThreadSafe {
   // All QuotaLimitHeuristic instances in this map are owned by us.
   typedef std::map<FunctionName, QuotaLimitHeuristics> FunctionHeuristicsMap;
 
-  // Purge resets all accumulated data (except |violation_errors_|) as if the
-  // service was just created. Called periodically so we don't consume an
-  // unbounded amount of memory while tracking quota.  Yes, this could mean an
-  // extension gets away with murder if it is timed right, but the extensions
-  // we are trying to limit are ones that consistently violate, so we'll
-  // converge to the correct set.
+  // Purge resets all accumulated data as if the service was just created.
+  // Called periodically so we don't consume an unbounded amount of memory
+  // while tracking quota.
   void Purge();
   void PurgeFunctionHeuristicsMap(FunctionHeuristicsMap* map);
   base::RepeatingTimer<QuotaService> purge_timer_;
@@ -81,12 +78,6 @@ class QuotaService : public base::NonThreadSafe {
   // Each heuristic will be evaluated and ANDed together to get a final answer.
   std::map<ExtensionId, FunctionHeuristicsMap> function_heuristics_;
 
-  // For now, as soon as an extension violates quota, we don't allow it to
-  // make any more requests to quota limited functions.  This provides a quick
-  // lookup for these extensions that is only stored in memory.
-  typedef std::map<std::string, std::string> ViolationErrorMap;
-  ViolationErrorMap violation_errors_;
-
   DISALLOW_COPY_AND_ASSIGN(QuotaService);
 };
 

extensions/browser/quota_service_unittest.cc

@@ -287,9 +287,8 @@ TEST_F(QuotaServiceTest, MultipleFunctionsDontInterfere) {
                              kStartTime + TimeDelta::FromSeconds(15)));
 }
 
-TEST_F(QuotaServiceTest, ViolatorsWillBeViolators) {
+TEST_F(QuotaServiceTest, ViolatorsWillBeForgiven) {
   scoped_refptr<MockFunction> f(new TimedLimitMockFunction("foo"));
-  scoped_refptr<MockFunction> g(new TimedLimitMockFunction("bar"));
   base::ListValue arg;
   arg.Append(new base::FundamentalValue(1));
   EXPECT_EQ("", service_->Assess(extension_a_, f.get(), &arg, kStartTime));
@@ -304,16 +303,30 @@ TEST_F(QuotaServiceTest, ViolatorsWillBeViolators) {
                              &arg,
                              kStartTime + TimeDelta::FromSeconds(15)));
 
-  // We don't allow this extension to use quota limited functions even if they
-  // wait a while.
-  EXPECT_NE(
-      "",
-      service_->Assess(
-          extension_a_, f.get(), &arg, kStartTime + TimeDelta::FromDays(1)));
-  EXPECT_NE(
-      "",
-      service_->Assess(
-          extension_a_, g.get(), &arg, kStartTime + TimeDelta::FromDays(1)));
+  // Waiting a while will give the extension access to the function again.
+  EXPECT_EQ("", service_->Assess(extension_a_, f.get(), &arg,
+                                 kStartTime + TimeDelta::FromDays(1)));
+
+  // And lose it again soon after.
+  EXPECT_EQ("", service_->Assess(extension_a_, f.get(), &arg,
+                                 kStartTime + TimeDelta::FromDays(1) +
+                                     TimeDelta::FromSeconds(10)));
+  EXPECT_NE("", service_->Assess(extension_a_, f.get(), &arg,
+                                 kStartTime + TimeDelta::FromDays(1) +
+                                     TimeDelta::FromSeconds(15)));
+
+  // Going further over quota should continue to fail within this time period,
+  // but still all restored later.
+  EXPECT_NE("", service_->Assess(extension_a_, f.get(), &arg,
+                                 kStartTime + TimeDelta::FromDays(1) +
+                                     TimeDelta::FromSeconds(20)));
+  EXPECT_NE("", service_->Assess(extension_a_, f.get(), &arg,
+                                 kStartTime + TimeDelta::FromDays(1) +
+                                     TimeDelta::FromSeconds(25)));
+
+  // Like now.
+  EXPECT_EQ("", service_->Assess(extension_a_, f.get(), &arg,
+                                 kStartTime + TimeDelta::FromDays(2)));
 }
 
 }  // namespace extensions