Make URLSearchParamsIterationSource not snapshot

Previously URLSearchParamsIterationSource iterated over a copy of URLSearchParams, but it is specified in the standard as iterating over the live list. Modify it to reference the original URLSearchParams object instead of making a copy. Also add web-platform tests for delete during foreach, and a chromium- specific test for GC of URLSearchParams during iteration. Remove failing expectations. BUG=677322 Change-Id: I8c53fd8dd9863fe1146c5b7849d4f08245b37bc1 Reviewed-on: https://chromium-review.googlesource.com/987839Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Adam Rice <ricea@chromium.org> Cr-Commit-Position: refs/heads/master@{#548363}

Make URLSearchParamsIterationSource not snapshot
Previously URLSearchParamsIterationSource iterated over a copy of URLSearchParams, but it is specified in the standard as iterating over the live list. Modify it to reference the original URLSearchParams object instead of making a copy. Also add web-platform tests for delete during foreach, and a chromium- specific test for GC of URLSearchParams during iteration. Remove failing expectations. BUG=677322 Change-Id: I8c53fd8dd9863fe1146c5b7849d4f08245b37bc1 Reviewed-on: https://chromium-review.googlesource.com/987839Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Adam Rice <ricea@chromium.org> Cr-Commit-Position: refs/heads/master@{#548363}
1c1627f7 · Adam Rice · Commit Bot · 4fc8b781 · 4fc8b781 · 1c1627f7
Commit 1c1627f7 authored Apr 05, 2018 by Adam Rice Committed by Commit Bot Apr 05, 2018
4 changed files
--- a/third_party/WebKit/LayoutTests/external/wpt/url/urlsearchparams-foreach-expected.txt
+++ b/third_party/WebKit/LayoutTests/external/wpt/url/urlsearchparams-foreach-expected.txt
-This is a testharness.js-based test.
-PASS ForEach Check
-FAIL For-of Check assert_array_equals: property 0, expected "y" but got "b"
-PASS empty
-Harness: the test ran to completion.
-
--- a/third_party/WebKit/LayoutTests/external/wpt/url/urlsearchparams-foreach.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/url/urlsearchparams-foreach.html
@@ -36,4 +36,48 @@ test(function() {
        assert_unreached(i);
    }
 }, "empty");
+
+test(function() {
+    const url = new URL("http://localhost/query?param0=0&param1=1&param2=2");
+    const searchParams = url.searchParams;
+    const seen = [];
+    for (const param of searchParams) {
+        if (param[0] === 'param0') {
+            searchParams.delete('param1');
+        }
+        seen.push(param);
+    }
+
+    assert_array_equals(seen[0], ["param0", "0"]);
+    assert_array_equals(seen[1], ["param2", "2"]);
+}, "delete next param during iteration");
+
+test(function() {
+    const url = new URL("http://localhost/query?param0=0&param1=1&param2=2");
+    const searchParams = url.searchParams;
+    const seen = [];
+    for (const param of searchParams) {
+        if (param[0] === 'param0') {
+            searchParams.delete('param0');
+            // 'param1=1' is now in the first slot, so the next iteration will see 'param2=2'.
+        } else {
+            seen.push(param);
+        }
+    }
+
+    assert_array_equals(seen[0], ["param2", "2"]);
+}, "delete current param during iteration");
+
+test(function() {
+    const url = new URL("http://localhost/query?param0=0&param1=1&param2=2");
+    const searchParams = url.searchParams;
+    const seen = [];
+    for (const param of searchParams) {
+        seen.push(param[0]);
+        searchParams.delete(param[0]);
+    }
+
+    assert_array_equals(seen, ["param0", "param2"], "param1 should not have been seen by the loop");
+    assert_equals(String(searchParams), "param1=1", "param1 should remain");
+}, "delete every param seen during iteration");
 </script>
--- a/third_party/WebKit/LayoutTests/fast/domurl/chromium/urlsearchparams-iterable-gc.html
+++ b/third_party/WebKit/LayoutTests/fast/domurl/chromium/urlsearchparams-iterable-gc.html
+<!doctype html>
+<html>
+<head>
+<meta charset="utf8">
+<link rel="help" href="https://url.spec.whatwg.org/#dom-urlsearchparams">
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<script>
+// This test is Chromium-specific because it involves GC.
+
+promise_test(async () => {
+  let params = new URLSearchParams('param0=0&param1=1');
+  const seen = [];
+  for (const param of params) {
+    const [key, value] = param;
+    if (key === 'param0') {
+      params = undefined;
+      // It takes two iterations of GC before Oilpan objects are collected.
+      for (let i = 0; i < 2; ++i) {
+        gc();
+        await new Promise(resolve => setTimeout(resolve, 0));
+      }
+    }
+    seen.push(key);
+  }
+  assert_array_equals(seen, ['param0', 'param1'],
+                      'both parameter should be seen');
+}, 'URLSearchParams should not be garbage collected while iterator exists');
+
+</script>
+</head>
+</html>
--- a/third_party/WebKit/Source/core/url/URLSearchParams.cpp
+++ b/third_party/WebKit/Source/core/url/URLSearchParams.cpp
@@ -18,24 +18,29 @@ namespace {
 class URLSearchParamsIterationSource final
    : public PairIterable<String, String>::IterationSource {
 public:
-  URLSearchParamsIterationSource(Vector<std::pair<String, String>> params)
+  explicit URLSearchParamsIterationSource(URLSearchParams* params)
      : params_(params), current_(0) {}

  bool Next(ScriptState*,
            String& key,
            String& value,
            ExceptionState&) override {
-    if (current_ >= params_.size())
+    if (current_ >= params_->Params().size())
      return false;

-    key = params_[current_].first;
-    value = params_[current_].second;
+    key = params_->Params()[current_].first;
+    value = params_->Params()[current_].second;
    current_++;
    return true;
  }

+  void Trace(blink::Visitor* visitor) {
+    visitor->Trace(params_);
+    PairIterable<String, String>::IterationSource::Trace(visitor);
+  }
+
 private:
-  Vector<std::pair<String, String>> params_;
+  Member<URLSearchParams> params_;
  size_t current_;
 };

@@ -253,7 +258,7 @@ scoped_refptr<EncodedFormData> URLSearchParams::ToEncodedFormData() const {
 PairIterable<String, String>::IterationSource* URLSearchParams::StartIteration(
    ScriptState*,
    ExceptionState&) {
-  return new URLSearchParamsIterationSource(params_);
+  return new URLSearchParamsIterationSource(this);
 }

 }  // namespace blink