Reset the nesting level for setInterval after completion.

third_party/WebKit/Source/core/frame/DOMTimerTest.cpp

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/frame/DOMTimer.h"
+
+#include <vector>
+
+#include "bindings/core/v8/ScriptController.h"
+#include "core/dom/Document.h"
+#include "core/layout/LayoutTestHelper.h"
+#include "gin/converter.h"
+#include "platform/testing/TestingPlatformSupport.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::DoubleNear;
+using testing::ElementsAreArray;
+using testing::Matcher;
+
+namespace blink {
+
+namespace {
+
+class DOMTimerTest : public RenderingTest {
+ public:
+  ScopedTestingPlatformSupport<TestingPlatformSupportWithMockScheduler>
+      platform_;

[alex clarke (OOO till 29th), 2017/02/14 16:22:18]

Sadly this is in blink, please rename to m_platform.  NB one day blink will be
reformatted fully to chromium style.

[Dan Elphick, 2017/02/14 17:11:03]

Done.

+
+  gin::Converter<std::vector<double>> converter;

[alex clarke (OOO till 29th), 2017/02/14 16:22:18]

please rename to m_converter

[Dan Elphick, 2017/02/14 17:11:03]

Done.

+
+  void SetUp() override {
+    platform_->setAutoAdvanceNowToPendingTasks(true);
+    // Advance timer manually as RenderingTest expects the time to be non-zero.
+    platform_->advanceClockSeconds(1.);
+    RenderingTest::SetUp();
+    // Advance timer again as otherwise the time between the first call to
+    // setInterval and it running will be off by 5us.
+    platform_->advanceClockSeconds(1);
+    document().settings()->setScriptEnabled(true);
+  }
+
+  v8::Local<v8::Value> EvalExpression(const char* expr) {
+    return document().frame()->script().executeScriptInMainWorldAndReturnValue(
+        ScriptSourceCode(expr));
+  }
+
+  void ExecuteScriptAndWaitUntilIdle(const char* scriptText) {
+    ScriptSourceCode script(scriptText);
+    document().frame()->script().executeScriptInMainWorld(script);
+    platform_->runUntilIdle();
+  }
+};
+
+const char* kSetTimeoutScriptText =

[alex clarke (OOO till 29th), 2017/02/14 16:22:18]

We are allowed to use C++11 string literals now which should make this more
readable:

const char* kSetTimeoutScriptText = R("
   var id;
   var last = performance.now();
   ...
   getTimeout(nestSetTimeouts, 1);");

[Dan Elphick, 2017/02/14 17:11:03]

Looks like check-webkit-style hasn't got the memo:(

check-webkit-style failed
  third_party/WebKit/Source/core/frame/DOMTimerTest.cpp:64:  Multi-line string
("...") found.  This lint script doesn't do well with such strings, and may give
bogus warnings.  They're ugly and unnecessary, and you should use concatenation
instead".  [readability/multiline_string] [5]

+    "var id;"
+    "var last = performance.now();"
+    "var times = [];"
+    "function nestSetTimeouts() {"
+    "  var current = performance.now();"
+    "  var elapsed = current - last;"
+    "  last = current;"
+    "  times.push(elapsed);"
+    "  if (times.length < 6) {"
+    "    setTimeout(nestSetTimeouts, 1);"
+    "  }"
+    "}"
+    "setTimeout(nestSetTimeouts, 1);";
+
+// Expected time between each iterator for setInterval(..., 1) or nested
+// setTimeout(..., 1) are 1, 1, 1, 1, 4, 4, ... as a minumum clamp of 4ms
+// is applied from the 5th iteration onwards.
+const std::vector<Matcher<double>> kExpectedTimings = {

[alex clarke (OOO till 29th), 2017/02/14 16:22:18]

AFAIK the style guide does not allow globals that dynamically allocate memory. I
think a plain C style array should work too. 

+    DoubleNear(1., 0.000001), DoubleNear(1., 0.000001),
+    DoubleNear(1., 0.000001), DoubleNear(1., 0.000001),
+    DoubleNear(4., 0.000001), DoubleNear(4., 0.000001),
+};
+
+TEST_F(DOMTimerTest, setTimeout_ClampsAfter4Nestings) {
+  v8::HandleScope scope(v8::Isolate::GetCurrent());
+
+  ExecuteScriptAndWaitUntilIdle(kSetTimeoutScriptText);
+
+  std::vector<double> times;
+  converter.FromV8(scope.GetIsolate(), EvalExpression("times"), &times);
+
+  EXPECT_THAT(times, ElementsAreArray(kExpectedTimings));
+}
+
+const char* kSetIntervalScriptText =
+    "var last = performance.now();"

[alex clarke (OOO till 29th), 2017/02/14 16:22:18]

Ditto

[Dan Elphick, 2017/02/14 17:11:03]

As above.

+    "var times = [];"
+    "var id = setInterval(function() {"
+    "  var current = performance.now();"
+    "  var elapsed = current - last;"
+    "  last = current;"
+    "  times.push(elapsed);"
+    "  if (times.length > 5) {"
+    "    clearInterval(id);"
+    "  }"
+    "}, 1);";
+
+TEST_F(DOMTimerTest, setInterval_ClampsAfter4Iterations) {
+  v8::HandleScope scope(v8::Isolate::GetCurrent());
+
+  ExecuteScriptAndWaitUntilIdle(kSetIntervalScriptText);
+
+  std::vector<double> times;
+  converter.FromV8(scope.GetIsolate(), EvalExpression("times"), &times);
+
+  EXPECT_THAT(times, ElementsAreArray(kExpectedTimings));
+}
+
+TEST_F(DOMTimerTest, setInterval_NestingResetsForLaterCalls) {
+  v8::HandleScope scope(v8::Isolate::GetCurrent());
+
+  ExecuteScriptAndWaitUntilIdle(kSetIntervalScriptText);
+
+  // Run the setIntervalScript again to verify that the clamp imposed for
+  // nesting beyond 4 levels is reset when setInterval is called again in the
+  // original scope but after the original setInterval has completed.
+  ExecuteScriptAndWaitUntilIdle(kSetIntervalScriptText);
+
+  std::vector<double> times;
+  converter.FromV8(scope.GetIsolate(), EvalExpression("times"), &times);
+
+  EXPECT_THAT(times, ElementsAreArray(kExpectedTimings));
+}
+
+}  // namespace
+
+}  // namespace blink