Remove the extension QuotaService::SustainedLimit and its last client, the

storage.sync MAX_SUSTAINED_WRITE_OPERATIONS_PER_MINUTE limit. I'll be relaxing
the quota over a few CLs and having 2 limiting mechanisms just complicates
things.

BUG=406406
R=zea@chromium.org

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

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

extensions/browser/api/storage/storage_api.cc

@@ -158,18 +158,6 @@ void GetModificationQuotaLimitHeuristics(QuotaLimitHeuristics* heuristics) {
       longLimitConfig,
       new QuotaLimitHeuristic::SingletonBucketMapper(),
       "MAX_WRITE_OPERATIONS_PER_HOUR"));
-
-  // A max of 10 operations per minute, sustained over 10 minutes.
-  QuotaLimitHeuristic::Config shortLimitConfig = {
-    // See storage.json for current value.
-    core_api::storage::sync::MAX_SUSTAINED_WRITE_OPERATIONS_PER_MINUTE,
-    base::TimeDelta::FromMinutes(1)
-  };
-  heuristics->push_back(new QuotaService::SustainedLimit(
-      base::TimeDelta::FromMinutes(10),
-      shortLimitConfig,
-      new QuotaLimitHeuristic::SingletonBucketMapper(),
-      "MAX_SUSTAINED_WRITE_OPERATIONS_PER_MINUTE"));
 };
 
 }  // namespace

extensions/browser/quota_service.cc

@@ -137,15 +137,6 @@ std::string QuotaLimitHeuristic::GetError() const {
   return extensions::ErrorUtils::FormatErrorMessage(kOverQuotaError, name_);
 }
 
-QuotaService::SustainedLimit::SustainedLimit(const base::TimeDelta& sustain,
-                                             const Config& config,
-                                             BucketMapper* map,
-                                             const std::string& name)
-    : QuotaLimitHeuristic(config, map, name),
-      repeat_exhaustion_allowance_(sustain.InSeconds() /
-                                   config.refill_interval.InSeconds()),
-      num_available_repeat_exhaustions_(repeat_exhaustion_allowance_) {}
-
 bool QuotaService::TimedLimit::Apply(Bucket* bucket,
                                      const base::TimeTicks& event_time) {
   if (event_time > bucket->expiration())
@@ -154,39 +145,4 @@ bool QuotaService::TimedLimit::Apply(Bucket* bucket,
   return bucket->DeductToken();
 }
 
-bool QuotaService::SustainedLimit::Apply(Bucket* bucket,
-                                         const base::TimeTicks& event_time) {
-  if (event_time > bucket->expiration()) {
-    // We reset state for this item and start over again if this request breaks
-    // the bad cycle that was previously being tracked.  This occurs if the
-    // state in the bucket expired recently (it has been long enough since the
-    // event that we don't care about the last event), but the bucket still has
-    // tokens (so pressure was not sustained over that time), OR we are more
-    // than 1 full refill interval away from the last event (so even if we used
-    // up all the tokens in the last bucket, nothing happened in the entire
-    // next refill interval, so it doesn't matter).
-    if (bucket->has_tokens() ||
-        event_time > bucket->expiration() + config().refill_interval) {
-      bucket->Reset(config(), event_time);
-      num_available_repeat_exhaustions_ = repeat_exhaustion_allowance_;
-    } else if (--num_available_repeat_exhaustions_ > 0) {
-      // The last interval was saturated with requests, and this is the first
-      // event in the next interval. If this happens
-      // repeat_exhaustion_allowance_ times, it's a violation. Reset the bucket
-      // state to start timing from the end of the last interval (and we'll
-      // deduct the token below) so we can detect this each time it happens.
-      bucket->Reset(config(), bucket->expiration());
-    } else {
-      // No allowances left; this request is a violation.
-      return false;
-    }
-  }
-
-  // We can go negative since we check has_tokens when we get to *next* bucket,
-  // and for the small interval all that matters is whether we used up all the
-  // tokens (which is true if num_tokens_ <= 0).
-  bucket->DeductToken();
-  return true;
-}
-
 }  // namespace extensions

extensions/browser/quota_service.h

@@ -45,7 +45,6 @@ class QuotaService : public base::NonThreadSafe {
   // Some concrete heuristics (declared below) that ExtensionFunctions can
   // use to help the service make decisions about quota violations.
   class TimedLimit;
-  class SustainedLimit;
 
   QuotaService();
   virtual ~QuotaService();
@@ -216,24 +215,6 @@ class QuotaService::TimedLimit : public QuotaLimitHeuristic {
   bool Apply(Bucket* bucket, const base::TimeTicks& event_time) override;
 };
 
-// A per-item heuristic to limit the number of events that can occur in a
-// period of time over a sustained longer interval. E.g "no more than two
-// events per minute, sustained over 10 minutes".
-class QuotaService::SustainedLimit : public QuotaLimitHeuristic {
- public:
-  SustainedLimit(const base::TimeDelta& sustain,
-                 const Config& config,
-                 BucketMapper* map,
-                 const std::string& name);
-  bool Apply(Bucket* bucket, const base::TimeTicks& event_time) override;
-
- private:
-  // Specifies how long exhaustion of buckets is allowed to continue before
-  // denying requests.
-  const int64 repeat_exhaustion_allowance_;
-  int64 num_available_repeat_exhaustions_;
-};
-
 }  // namespace extensions
 
 #endif  // EXTENSIONS_BROWSER_QUOTA_SERVICE_H_

extensions/browser/quota_service_unittest.cc

@@ -21,7 +21,6 @@ typedef QuotaLimitHeuristic::Bucket Bucket;
 typedef QuotaLimitHeuristic::Config Config;
 typedef QuotaLimitHeuristic::BucketList BucketList;
 typedef QuotaService::TimedLimit TimedLimit;
-typedef QuotaService::SustainedLimit SustainedLimit;
 
 namespace {
 
@@ -88,24 +87,6 @@ class TimedLimitMockFunction : public MockFunction {
   ~TimedLimitMockFunction() override {}
 };
 
-class ChainedLimitsMockFunction : public MockFunction {
- public:
-  explicit ChainedLimitsMockFunction(const std::string& name)
-      : MockFunction(name) {}
-  void GetQuotaLimitHeuristics(
-      QuotaLimitHeuristics* heuristics) const override {
-    // No more than 2 per minute sustained over 5 minutes.
-    heuristics->push_back(new SustainedLimit(
-        TimeDelta::FromMinutes(5), k2PerMinute, new Mapper(), kGenericName));
-    // No more than 20 per hour.
-    heuristics->push_back(
-        new TimedLimit(k20PerHour, new Mapper(), kGenericName));
-  }
-
- private:
-  ~ChainedLimitsMockFunction() override {}
-};
-
 class FrozenMockFunction : public MockFunction {
  public:
   explicit FrozenMockFunction(const std::string& name) : MockFunction(name) {}
@@ -189,31 +170,6 @@ TEST_F(QuotaLimitHeuristicTest, Timed) {
   EXPECT_FALSE(lim.Apply(&b, k1MinuteAfterStart + TimeDelta::FromSeconds(3)));
 }
 
-TEST_F(QuotaLimitHeuristicTest, Sustained) {
-  SustainedLimit lim(
-      TimeDelta::FromMinutes(5), k2PerMinute, new MockMapper(), kGenericName);
-  Bucket bucket;
-
-  bucket.Reset(k2PerMinute, kStartTime);
-  DoMoreThan2PerMinuteFor5Minutes(kStartTime, &lim, &bucket, -1);
-  // This straw breaks the camel's back.
-  EXPECT_FALSE(lim.Apply(&bucket, kStartTime + TimeDelta::FromMinutes(6)));
-
-  // The heuristic resets itself on a safe request.
-  EXPECT_TRUE(lim.Apply(&bucket, kStartTime + TimeDelta::FromDays(1)));
-
-  // Do the same as above except don't exhaust final bucket.
-  bucket.Reset(k2PerMinute, kStartTime);
-  DoMoreThan2PerMinuteFor5Minutes(kStartTime, &lim, &bucket, -1);
-  EXPECT_TRUE(lim.Apply(&bucket, kStartTime + TimeDelta::FromMinutes(7)));
-
-  // Do the same as above except don't exhaust the 3rd (w.l.o.g) bucket.
-  bucket.Reset(k2PerMinute, kStartTime);
-  DoMoreThan2PerMinuteFor5Minutes(kStartTime, &lim, &bucket, 3);
-  // If the 3rd bucket were exhausted, this would fail (see first test).
-  EXPECT_TRUE(lim.Apply(&bucket, kStartTime + TimeDelta::FromMinutes(6)));
-}
-
 TEST_F(QuotaServiceTest, NoHeuristic) {
   scoped_refptr<MockFunction> f(new MockFunction("foo"));
   base::ListValue args;
@@ -298,59 +254,6 @@ TEST_F(QuotaServiceTest, SingleHeuristic) {
                              kStartTime + TimeDelta::FromSeconds(30)));
 }
 
-TEST_F(QuotaServiceTest, ChainedHeuristics) {
-  scoped_refptr<MockFunction> f(new ChainedLimitsMockFunction("foo"));
-  base::ListValue args;
-  args.Append(new base::FundamentalValue(1));
-
-  // First, test that the low limit can be avoided but the higher one is hit.
-  // One event per minute for 20 minutes comes in under the sustained limit,
-  // but is equal to the timed limit.
-  for (int i = 0; i < 20; i++) {
-    EXPECT_EQ(
-        "",
-        service_->Assess(extension_a_,
-                         f.get(),
-                         &args,
-                         kStartTime + TimeDelta::FromSeconds(10 + i * 60)));
-  }
-
-  // This will bring us to 21 events in an hour, which is a violation.
-  EXPECT_NE("",
-            service_->Assess(extension_a_,
-                             f.get(),
-                             &args,
-                             kStartTime + TimeDelta::FromMinutes(30)));
-
-  // Now, check that we can still hit the lower limit.
-  for (int i = 0; i < 5; i++) {
-    EXPECT_EQ(
-        "",
-        service_->Assess(extension_b_,
-                         f.get(),
-                         &args,
-                         kStartTime + TimeDelta::FromSeconds(10 + i * 60)));
-    EXPECT_EQ(
-        "",
-        service_->Assess(extension_b_,
-                         f.get(),
-                         &args,
-                         kStartTime + TimeDelta::FromSeconds(15 + i * 60)));
-    EXPECT_EQ(
-        "",
-        service_->Assess(extension_b_,
-                         f.get(),
-                         &args,
-                         kStartTime + TimeDelta::FromSeconds(20 + i * 60)));
-  }
-
-  EXPECT_NE("",
-            service_->Assess(extension_b_,
-                             f.get(),
-                             &args,
-                             kStartTime + TimeDelta::FromMinutes(6)));
-}
-
 TEST_F(QuotaServiceTest, MultipleFunctionsDontInterfere) {
   scoped_refptr<MockFunction> f(new TimedLimitMockFunction("foo"));
   scoped_refptr<MockFunction> g(new TimedLimitMockFunction("bar"));

extensions/common/api/storage.json

@@ -193,8 +193,9 @@
             "description": "The maximum number of <code>set</code>, <code>remove</code>, or <code>clear</code> operations that can be performed each hour. Updates that would cause this limit to be exceeded fail immediately and set $(ref:runtime.lastError)."
           },
           "MAX_SUSTAINED_WRITE_OPERATIONS_PER_MINUTE": {
-            "value": 10,
-            "description": "The maximum number of <code>set</code>, <code>remove</code>, or <code>clear</code> operations that can be performed each minute, sustained over 10 minutes. Updates that would cause this limit to be exceeded fail immediately and set $(ref:runtime.lastError)."
+            "value": 1000000,
+            "deprecated": "The storage.sync API no longer has a sustained write operation quota.",
+            "description": ""
           }
         }
       },