bindings: Improve ScriptIterator's API.

* Reorder arguments in ScriptIterator's constructor that takes a
  v8::Isolate.
  In general, functions that take a v8::Isolate* have it as the first
  argument. Make ScriptIterator's constructor follow suit for
  consistency.
* Replace the ScriptIterator(nullptr_t) constructor with a default one.
  Taking a nullptr (as well as not making the constructor explicit)
  never made much sense in the first place.

Change-Id: I19873f5e6c1996279c76029aa3303fe50a890023
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1928778Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#717613}

third_party/blink/renderer/bindings/core/v8/script_iterator.cc

@@ -19,22 +19,22 @@ ScriptIterator ScriptIterator::FromIterable(v8::Isolate* isolate,
   const v8::Local<v8::Function> iterator_method =
       GetEsIteratorMethod(isolate, value, exception_state);
   if (exception_state.HadException())
-    return ScriptIterator(nullptr);
+    return ScriptIterator();
   if (iterator_method.IsEmpty())
-    return ScriptIterator(nullptr);
+    return ScriptIterator();
 
   // Use the method returned above to invoke the GetIterator(V, sync, method)
   // abstract ES operation.
   const v8::Local<v8::Object> iterator =
       GetEsIteratorWithMethod(isolate, iterator_method, value, exception_state);
   if (exception_state.HadException())
-    return ScriptIterator(nullptr);
+    return ScriptIterator();
 
-  return ScriptIterator(iterator, isolate);
+  return ScriptIterator(isolate, iterator);
 }
 
-ScriptIterator::ScriptIterator(v8::Local<v8::Object> iterator,
-                               v8::Isolate* isolate)
+ScriptIterator::ScriptIterator(v8::Isolate* isolate,
+                               v8::Local<v8::Object> iterator)
     : isolate_(isolate),
       iterator_(iterator),
       next_key_(V8AtomicString(isolate, "next")),

third_party/blink/renderer/bindings/core/v8/script_iterator.h

@@ -65,9 +65,12 @@ class CORE_EXPORT ScriptIterator {
                                      v8::Local<v8::Object>,
                                      ExceptionState&);
 
-  ScriptIterator(std::nullptr_t) : isolate_(nullptr), done_(true) {}
+  // Constructs a ScriptIterator from an ES object that implements the iterator
+  // protocol: |iterator| is supposed to have a next() method that returns an
+  // object with two properties, "done" and "value".
+  ScriptIterator(v8::Isolate*, v8::Local<v8::Object> iterator);
 
-  ScriptIterator(v8::Local<v8::Object> iterator, v8::Isolate*);
+  ScriptIterator() = default;
 
   ScriptIterator(ScriptIterator&&) noexcept = default;
   ScriptIterator& operator=(ScriptIterator&&) noexcept = default;
@@ -85,12 +88,12 @@ class CORE_EXPORT ScriptIterator {
   v8::MaybeLocal<v8::Value> GetValue() { return value_; }
 
  private:
-  v8::Isolate* isolate_;
+  v8::Isolate* isolate_ = nullptr;
   v8::Local<v8::Object> iterator_;
   v8::Local<v8::String> next_key_;
   v8::Local<v8::String> done_key_;
   v8::Local<v8::String> value_key_;
-  bool done_;
+  bool done_ = true;
   v8::MaybeLocal<v8::Value> value_;
 };
 

third_party/blink/renderer/core/testing/dictionary_test.cc

@@ -21,17 +21,17 @@ ScriptIterator GetIterator(const Dictionary& iterable,
   v8::Isolate* isolate = iterable.GetIsolate();
   if (!iterable.Get(v8::Symbol::GetIterator(isolate), iterator_getter) ||
       !iterator_getter->IsFunction()) {
-    return nullptr;
+    return ScriptIterator();
   }
   v8::Local<v8::Value> iterator;
   if (!V8ScriptRunner::CallFunction(
            v8::Local<v8::Function>::Cast(iterator_getter), execution_context,
            iterable.V8Value(), 0, nullptr, isolate)
            .ToLocal(&iterator))
-    return nullptr;
+    return ScriptIterator();
   if (!iterator->IsObject())
-    return nullptr;
-  return ScriptIterator(v8::Local<v8::Object>::Cast(iterator), isolate);
+    return ScriptIterator();
+  return ScriptIterator(isolate, v8::Local<v8::Object>::Cast(iterator));
 }
 }  // namespace