url: improve URLSearchParams spec compliance

- Make URLSearchParams constructor spec-compliant
- Strip leading `?` in URL#search's setter
- Spec-compliant iterable interface
- More precise handling of update steps as mandated by the spec
- Add class strings to URLSearchParams objects and their prototype
- Make sure `this instanceof URLSearchParams` in methods

Also included are relevant tests from W3C's Web Platform Tests
(https://github.com/w3c/web-platform-tests/tree/master/url).

Fixes: nodejs#9302
PR-URL: nodejs#9484
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/internal/url.js

@@ -20,6 +20,11 @@ const kHost = Symbol('host');
 const kPort = Symbol('port');
 const kDomain = Symbol('domain');
 
+// https://tc39.github.io/ecma262/#sec-%iteratorprototype%-object
+const IteratorPrototype = Object.getPrototypeOf(
+  Object.getPrototypeOf([][Symbol.iterator]())
+);
+
 function StorageObject() {}
 StorageObject.prototype = Object.create(null);
 
@@ -101,7 +106,8 @@ class URL {
         this[context].query = query;
         this[context].fragment = fragment;
         this[context].host = host;
-        this[searchParams] = new URLSearchParams(this);
+        this[searchParams] = new URLSearchParams(query);
+        this[searchParams][context] = this;
       });
   }
 
@@ -318,8 +324,31 @@ class URL {
   }
 
   set search(search) {
-    update(this, search);
-    this[searchParams][searchParams] = querystring.parse(this.search);
+    search = String(search);
+    if (search[0] === '?') search = search.slice(1);
+    if (!search) {
+      this[context].query = null;
+      this[context].flags &= ~binding.URL_FLAGS_HAS_QUERY;
+      this[searchParams][searchParams] = {};
+      return;
+    }
+    this[context].query = '';
+    binding.parse(search,
+                  binding.kQuery,
+                  null,
+                  this[context],
+      (flags, protocol, username, password,
+       host, port, path, query, fragment) => {
+        if (flags & binding.URL_FLAGS_FAILED)
+          return;
+        if (query) {
+          this[context].query = query;
+          this[context].flags |= binding.URL_FLAGS_HAS_QUERY;
+        } else {
+          this[context].flags &= ~binding.URL_FLAGS_HAS_QUERY;
+        }
+      });
+    this[searchParams][searchParams] = querystring.parse(search);
   }
 
   get hash() {
@@ -493,105 +522,273 @@ function encodeAuth(str) {
   return out;
 }
 
-function update(url, search) {
-  search = String(search);
-  if (!search) {
-    url[context].query = null;
-    url[context].flags &= ~binding.URL_FLAGS_HAS_QUERY;
+function update(url, params) {
+  if (!url)
     return;
+
+  url[context].query = params.toString();
+}
+
+function getSearchParamPairs(target) {
+  const obj = target[searchParams];
+  const keys = Object.keys(obj);
+  const values = [];
+  for (var i = 0; i < keys.length; i++) {
+    const name = keys[i];
+    const value = obj[name];
+    if (Array.isArray(value)) {
+      for (const item of value)
+        values.push([name, item]);
+    } else {
+      values.push([name, value]);
+    }
   }
-  if (search[0] === '?') search = search.slice(1);
-  url[context].query = '';
-  binding.parse(search,
-                binding.kQuery,
-                null,
-                url[context],
-    (flags, protocol, username, password,
-     host, port, path, query, fragment) => {
-      if (flags & binding.URL_FLAGS_FAILED)
-        return;
-      if (query) {
-        url[context].query = query;
-        url[context].flags |= binding.URL_FLAGS_HAS_QUERY;
-      } else {
-        url[context].flags &= ~binding.URL_FLAGS_HAS_QUERY;
-      }
-    });
+  return values;
 }
 
 class URLSearchParams {
-  constructor(url) {
-    this[context] = url;
-    this[searchParams] = querystring.parse(url[context].search || '');
+  constructor(init = '') {
+    if (init instanceof URLSearchParams) {
+      const childParams = init[searchParams];
+      this[searchParams] = Object.assign(Object.create(null), childParams);
+    } else {
+      init = String(init);
+      if (init[0] === '?') init = init.slice(1);
+      this[searchParams] = querystring.parse(init);
+    }
+
+    // "associated url object"
+    this[context] = null;
+
+    // Class string for an instance of URLSearchParams. This is different from
+    // the class string of the prototype object (set below).
+    Object.defineProperty(this, Symbol.toStringTag, {
+      value: 'URLSearchParams',
+      writable: false,
+      enumerable: false,
+      configurable: true
+    });
   }
 
   append(name, value) {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+    if (arguments.length < 2) {
+      throw new TypeError(
+        'Both `name` and `value` arguments need to be specified');
+    }
+
     const obj = this[searchParams];
     name = String(name);
     value = String(value);
     var existing = obj[name];
-    if (!existing) {
+    if (existing === undefined) {
       obj[name] = value;
     } else if (Array.isArray(existing)) {
       existing.push(value);
     } else {
       obj[name] = [existing, value];
     }
-    update(this[context], querystring.stringify(obj));
+    update(this[context], this);
   }
 
   delete(name) {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+    if (arguments.length < 1) {
+      throw new TypeError('The `name` argument needs to be specified');
+    }
+
     const obj = this[searchParams];
     name = String(name);
     delete obj[name];
-    update(this[context], querystring.stringify(obj));
+    update(this[context], this);
   }
 
   set(name, value) {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+    if (arguments.length < 2) {
+      throw new TypeError(
+        'Both `name` and `value` arguments need to be specified');
+    }
+
     const obj = this[searchParams];
     name = String(name);
     value = String(value);
     obj[name] = value;
-    update(this[context], querystring.stringify(obj));
+    update(this[context], this);
   }
 
   get(name) {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+    if (arguments.length < 1) {
+      throw new TypeError('The `name` argument needs to be specified');
+    }
+
     const obj = this[searchParams];
     name = String(name);
     var value = obj[name];
-    return Array.isArray(value) ? value[0] : value;
+    return value === undefined ? null : Array.isArray(value) ? value[0] : value;
   }
 
   getAll(name) {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+    if (arguments.length < 1) {
+      throw new TypeError('The `name` argument needs to be specified');
+    }
+
     const obj = this[searchParams];
     name = String(name);
     var value = obj[name];
     return value === undefined ? [] : Array.isArray(value) ? value : [value];
   }
 
   has(name) {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+    if (arguments.length < 1) {
+      throw new TypeError('The `name` argument needs to be specified');
+    }
+
     const obj = this[searchParams];
     name = String(name);
     return name in obj;
   }
 
-  *[Symbol.iterator]() {
-    const obj = this[searchParams];
-    for (const name in obj) {
-      const value = obj[name];
-      if (Array.isArray(value)) {
-        for (const item of value)
-          yield [name, item];
-      } else {
-        yield [name, value];
-      }
+  // https://heycam.github.io/webidl/#es-iterators
+  // Define entries here rather than [Symbol.iterator] as the function name
+  // must be set to `entries`.
+  entries() {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
     }
+
+    return createSearchParamsIterator(this, 'key+value');
   }
 
+  forEach(callback, thisArg = undefined) {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+    if (arguments.length < 1) {
+      throw new TypeError('The `callback` argument needs to be specified');
+    }
+
+    let pairs = getSearchParamPairs(this);
+
+    var i = 0;
+    while (i < pairs.length) {
+      const [key, value] = pairs[i];
+      callback.call(thisArg, value, key, this);
+      pairs = getSearchParamPairs(this);
+      i++;
+    }
+  }
+
+  // https://heycam.github.io/webidl/#es-iterable
+  keys() {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+
+    return createSearchParamsIterator(this, 'key');
+  }
+
+  values() {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+
+    return createSearchParamsIterator(this, 'value');
+  }
+
+  // https://url.spec.whatwg.org/#urlsearchparams-stringification-behavior
   toString() {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+
     return querystring.stringify(this[searchParams]);
   }
 }
+// https://heycam.github.io/webidl/#es-iterable-entries
+URLSearchParams.prototype[Symbol.iterator] = URLSearchParams.prototype.entries;
+Object.defineProperty(URLSearchParams.prototype, Symbol.toStringTag, {
+  value: 'URLSearchParamsPrototype',
+  writable: false,
+  enumerable: false,
+  configurable: true
+});
+
+// https://heycam.github.io/webidl/#dfn-default-iterator-object
+function createSearchParamsIterator(target, kind) {
+  const iterator = Object.create(URLSearchParamsIteratorPrototype);
+  iterator[context] = {
+    target,
+    kind,
+    index: 0
+  };
+  return iterator;
+}
+
+// https://heycam.github.io/webidl/#dfn-iterator-prototype-object
+const URLSearchParamsIteratorPrototype = Object.setPrototypeOf({
+  next() {
+    if (!this ||
+        Object.getPrototypeOf(this) !== URLSearchParamsIteratorPrototype) {
+      throw new TypeError('Value of `this` is not a URLSearchParamsIterator');
+    }
+
+    const {
+      target,
+      kind,
+      index
+    } = this[context];
+    const values = getSearchParamPairs(target);
+    const len = values.length;
+    if (index >= len) {
+      return {
+        value: undefined,
+        done: true
+      };
+    }
+
+    const pair = values[index];
+    this[context].index = index + 1;
+
+    let result;
+    if (kind === 'key') {
+      result = pair[0];
+    } else if (kind === 'value') {
+      result = pair[1];
+    } else {
+      result = pair;
+    }
+
+    return {
+      value: result,
+      done: false
+    };
+  }
+}, IteratorPrototype);
+
+// Unlike interface and its prototype object, both default iterator object and
+// iterator prototype object of an interface have the same class string.
+Object.defineProperty(URLSearchParamsIteratorPrototype, Symbol.toStringTag, {
+  value: 'URLSearchParamsIterator',
+  writable: false,
+  enumerable: false,
+  configurable: true
+});
 
 URL.originFor = function(url) {
   if (!(url instanceof URL))