Browse Source

LibJS+LibWeb: Clear exceptions after call'ing JavaScript functions

Decorated Interpreter::call() with [[nodiscard]] to provoke thinking
about the returned value at each call site. This is definitely not
perfect and we should really start thinking about slimming down the
public-facing LibJS interpreter API.

Fixes #3136.
Andreas Kling 4 years ago
parent
commit
bbe2d4a2d9

+ 1 - 1
Libraries/LibJS/Interpreter.h

@@ -118,7 +118,7 @@ public:
     void enter_scope(const ScopeNode&, ArgumentVector, ScopeType, GlobalObject&);
     void exit_scope(const ScopeNode&);
 
-    Value call(Function&, Value this_value, Optional<MarkedValueList> arguments = {});
+    [[nodiscard]] Value call(Function&, Value this_value, Optional<MarkedValueList> arguments = {});
     Value construct(Function&, Function& new_target, Optional<MarkedValueList> arguments, GlobalObject&);
 
     CallFrame& push_call_frame()

+ 2 - 1
Libraries/LibJS/Runtime/Accessor.h

@@ -65,7 +65,8 @@ public:
             return;
         MarkedValueList arguments(interpreter().heap());
         arguments.append(setter_value);
-        interpreter().call(*m_setter, this_value, move(arguments));
+        // FIXME: It might be nice if we had a way to communicate to our caller if an exception happened after this.
+        (void)interpreter().call(*m_setter, this_value, move(arguments));
     }
 
     void visit_children(Cell::Visitor& visitor) override

+ 1 - 1
Libraries/LibJS/Runtime/Object.cpp

@@ -737,7 +737,7 @@ bool Object::put(const PropertyName& property_name, Value value, Value receiver)
         }
         object = object->prototype();
         if (interpreter().exception())
-            return {};
+            return false;
     }
     return put_own_property(*this, string_or_symbol, value, default_attributes, PutOwnPropertyMode::Put);
 }

+ 4 - 1
Libraries/LibWeb/DOM/Node.cpp

@@ -127,7 +127,10 @@ void Node::dispatch_event(NonnullRefPtr<Event> event)
             auto* event_wrapper = wrap(global_object, *event);
             JS::MarkedValueList arguments(global_object.heap());
             arguments.append(event_wrapper);
-            document().interpreter().call(function, this_value, move(arguments));
+            auto& interpreter = document().interpreter();
+            (void)interpreter.call(function, this_value, move(arguments));
+            if (interpreter.exception())
+                interpreter.clear_exception();
         }
     }
 

+ 6 - 2
Libraries/LibWeb/DOM/Window.cpp

@@ -94,7 +94,9 @@ void Window::timer_did_fire(Badge<Timer>, Timer& timer)
     }
 
     auto& interpreter = wrapper()->interpreter();
-    interpreter.call(timer.callback(), wrapper());
+    (void)interpreter.call(timer.callback(), wrapper());
+    if (interpreter.exception())
+        interpreter.clear_exception();
 }
 
 i32 Window::allocate_timer_id(Badge<Timer>)
@@ -123,7 +125,9 @@ i32 Window::request_animation_frame(JS::Function& callback)
         JS::MarkedValueList arguments(interpreter.heap());
         arguments.append(JS::Value(fake_timestamp));
         fake_timestamp += 10;
-        interpreter.call(function, {}, move(arguments));
+        (void)interpreter.call(function, {}, move(arguments));
+        if (interpreter.exception())
+            interpreter.clear_exception();
         GUI::DisplayLink::unregister_callback(link_id);
     });
 

+ 4 - 1
Libraries/LibWeb/DOM/XMLHttpRequest.cpp

@@ -98,7 +98,10 @@ void XMLHttpRequest::dispatch_event(NonnullRefPtr<DOM::Event> event)
             auto* this_value = wrap(global_object, *this);
             JS::MarkedValueList arguments(global_object.heap());
             arguments.append(wrap(global_object, *event));
-            function.interpreter().call(function, this_value, move(arguments));
+            auto& interpreter = function.interpreter();
+            (void)interpreter.call(function, this_value, move(arguments));
+            if (interpreter.exception())
+                interpreter.clear_exception();
         }
     }
 }

+ 6 - 2
Userland/test-web.cpp

@@ -339,7 +339,9 @@ JSFileResult TestRunner::run_file_test(const String& test_path)
             new_interpreter.run(new_interpreter.global_object(), *file_program.value());
 
             auto& before_initial_page_load = new_interpreter.get_variable("__BeforeInitialPageLoad__", new_interpreter.global_object()).as_function();
-            new_interpreter.call(before_initial_page_load, JS::js_undefined());
+            (void)new_interpreter.call(before_initial_page_load, JS::js_undefined());
+            if (new_interpreter.exception())
+                new_interpreter.clear_exception();
 
             // Now parse the HTML page.
             parser.run(page_to_load);
@@ -347,7 +349,9 @@ JSFileResult TestRunner::run_file_test(const String& test_path)
 
             // Finally run the test by calling "__AfterInitialPageLoad__"
             auto& after_initial_page_load = new_interpreter.get_variable("__AfterInitialPageLoad__", new_interpreter.global_object()).as_function();
-            new_interpreter.call(after_initial_page_load, JS::js_undefined());
+            (void)new_interpreter.call(after_initial_page_load, JS::js_undefined());
+            if (new_interpreter.exception())
+                new_interpreter.clear_exception();
 
             auto test_json = get_test_results(new_interpreter);
             if (!test_json.has_value()) {