LibJS: Avoid creating String object wrappers in iterator helpers

This is a normative change in the Iterator Helpers spec. See:
3e275cf

Userland/Libraries/LibJS/Runtime/Iterator.cpp

@@ -39,11 +39,15 @@ ThrowCompletionOr<IteratorRecord> get_iterator_direct(VM& vm, Object& object)
     return iterator_record;
 }
 
-ThrowCompletionOr<IteratorRecord> get_iterator_flattenable(VM& vm, Value object)
+// 2.1.2 GetIteratorFlattenable ( obj, stringHandling ), https://tc39.es/proposal-iterator-helpers/#sec-getiteratorflattenable
+ThrowCompletionOr<IteratorRecord> get_iterator_flattenable(VM& vm, Value object, StringHandling string_handling)
 {
-    // 1. If obj is not an Object, throw a TypeError exception.
+    // 1. If obj is not an Object, then
-    if (!object.is_object())
+    if (!object.is_object()) {
+        // a. If stringHandling is reject-strings or obj is not a String, throw a TypeError exception.
+        if (string_handling == StringHandling::RejectStrings || !object.is_string())
             return vm.throw_completion<TypeError>(ErrorType::NotAnObject, "obj"sv);
+    }
 
     // 2. Let method be ? GetMethod(obj, @@iterator).
     auto method = TRY(object.get_method(vm, vm.well_known_symbol_iterator()));

Userland/Libraries/LibJS/Runtime/Iterator.h

@@ -35,7 +35,12 @@ private:
     IteratorRecord m_iterated; // [[Iterated]]
 };
 
+enum class StringHandling {
+    IterateStrings,
+    RejectStrings,
+};
+
 ThrowCompletionOr<IteratorRecord> get_iterator_direct(VM&, Object&);
-ThrowCompletionOr<IteratorRecord> get_iterator_flattenable(VM&, Value);
+ThrowCompletionOr<IteratorRecord> get_iterator_flattenable(VM&, Value, StringHandling);
 
 }

Userland/Libraries/LibJS/Runtime/IteratorConstructor.cpp

@@ -66,27 +66,23 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorConstructor::from)
 
     auto object = vm.argument(0);
 
-    // 1. If O is a String, set O to ! ToObject(O).
+    // 1. Let iteratorRecord be ? GetIteratorFlattenable(O, iterate-strings).
-    if (object.is_string())
+    auto iterator_record = TRY(get_iterator_flattenable(vm, object, StringHandling::IterateStrings));
-        object = MUST_OR_THROW_OOM(object.to_object(vm));
 
-    // 2. Let iteratorRecord be ? GetIteratorFlattenable(O).
+    // 2. Let hasInstance be ? OrdinaryHasInstance(%Iterator%, iteratorRecord.[[Iterator]]).
-    auto iterator_record = TRY(get_iterator_flattenable(vm, object));
-
-    // 3. Let hasInstance be ? OrdinaryHasInstance(%Iterator%, iteratorRecord.[[Iterator]]).
     auto has_instance = TRY(ordinary_has_instance(vm, iterator_record.iterator, realm.intrinsics().iterator_constructor()));
 
-    // 4. If hasInstance is true, then
+    // 3. If hasInstance is true, then
     if (has_instance.is_boolean() && has_instance.as_bool()) {
         // a. Return iteratorRecord.[[Iterator]].
         return iterator_record.iterator;
     }
 
-    // 5. Let wrapper be OrdinaryObjectCreate(%WrapForValidIteratorPrototype%, « [[Iterated]] »).
+    // 4. Let wrapper be OrdinaryObjectCreate(%WrapForValidIteratorPrototype%, « [[Iterated]] »).
-    // 6. Set wrapper.[[Iterated]] to iteratorRecord.
+    // 5. Set wrapper.[[Iterated]] to iteratorRecord.
     auto wrapper = MUST_OR_THROW_OOM(Iterator::create(realm, realm.intrinsics().wrap_for_valid_iterator_prototype(), move(iterator_record)));
 
-    // 7. Return wrapper.
+    // 6. Return wrapper.
     return wrapper;
 }
 

Userland/Libraries/LibJS/Runtime/IteratorPrototype.cpp

@@ -359,8 +359,8 @@ private:
         if (mapped.is_error())
             return iterator.close_result(mapped.release_error());
 
-        // vi. Let innerIterator be Completion(GetIteratorFlattenable(mapped)).
+        // vi. Let innerIterator be Completion(GetIteratorFlattenable(mapped, reject-strings)).
-        auto inner_iterator = get_iterator_flattenable(vm, mapped.release_value());
+        auto inner_iterator = get_iterator_flattenable(vm, mapped.release_value(), StringHandling::RejectStrings);
 
         // vii. IfAbruptCloseIterator(innerIterator, iterated).
         if (inner_iterator.is_error())

Userland/Libraries/LibJS/Tests/builtins/Iterator/Iterator.from.js

@@ -60,6 +60,22 @@ describe("normal behavior", () => {
         expect(result.done).toBeTrue();
     });
 
+    test("does not coerce strings to objects", () => {
+        const stringIterator = String.prototype[Symbol.iterator];
+        let observedType = null;
+
+        Object.defineProperty(String.prototype, Symbol.iterator, {
+            get() {
+                "use strict";
+                observedType = typeof this;
+                return stringIterator;
+            },
+        });
+
+        Iterator.from("ab");
+        expect(observedType).toBe("string");
+    });
+
     test("create Iterator from generator", () => {
         function* generator() {
             yield 1;