LibWeb: Cache and reuse resolved IDL imports instead of rejecting them

This ensures that transitive imports succeed even if they were directly
imported beforehand.

3 files changed, 27 insertions, 28 deletions

diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLParser.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLParser.cpp
index 8c1fe69851..f009e16c6f 100644
--- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLParser.cpp
+++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLParser.cpp
@@ -76,7 +76,7 @@ static String convert_enumeration_value_to_cpp_enum_member(String const& value,
 
 namespace IDL {
 
-HashTable<String> Parser::s_all_imported_paths {};
+HashMap<String, NonnullRefPtr<Interface>> Parser::s_resolved_imports {};
 
 void Parser::assert_specific(char ch)
 {
@@ -124,17 +124,20 @@ HashMap<String, String> Parser::parse_extended_attributes()
     return extended_attributes;
 }
 
-Optional<NonnullOwnPtr<Interface>> Parser::resolve_import(auto path)
+static HashTable<String> import_stack;
+Optional<NonnullRefPtr<Interface>> Parser::resolve_import(auto path)
 {
     auto include_path = LexicalPath::join(import_base_path, path).string();
     if (!Core::File::exists(include_path))
         report_parsing_error(String::formatted("{}: No such file or directory", include_path), filename, input, lexer.tell());
 
     auto real_path = Core::File::real_path_for(include_path);
-    if (s_all_imported_paths.contains(real_path))
-        return {};
+    if (s_resolved_imports.contains(real_path))
+        return s_resolved_imports.find(real_path)->value;
 
-    s_all_imported_paths.set(real_path);
+    if (import_stack.contains(real_path))
+        report_parsing_error(String::formatted("Circular import detected: {}", include_path), filename, input, lexer.tell());
+    import_stack.set(real_path);
 
     auto file_or_error = Core::File::open(real_path, Core::OpenMode::ReadOnly);
     if (file_or_error.is_error())
@@ -142,8 +145,9 @@ Optional<NonnullOwnPtr<Interface>> Parser::resolve_import(auto path)
 
     auto data = file_or_error.value()->read_all();
     auto result = Parser(real_path, data, import_base_path).parse();
-    if (result->will_generate_code())
-        required_imported_paths.set(real_path);
+    import_stack.remove(real_path);
+
+    s_resolved_imports.set(real_path, result);
     return result;
 }
 
@@ -705,7 +709,7 @@ void Parser::parse_dictionary(Interface& interface)
 
 void Parser::parse_interface_mixin(Interface& interface)
 {
-    auto mixin_interface = make<Interface>();
+    auto mixin_interface = make_ref_counted<Interface>();
     mixin_interface->module_own_path = interface.module_own_path;
     mixin_interface->is_mixin = true;
 
@@ -813,15 +817,15 @@ void resolve_function_typedefs(Interface& interface, FunctionType& function)
     resolve_parameters_typedefs(interface, function.parameters);
 }
 
-NonnullOwnPtr<Interface> Parser::parse()
+NonnullRefPtr<Interface> Parser::parse()
 {
     auto this_module = Core::File::real_path_for(filename);
-    s_all_imported_paths.set(this_module);
 
-    auto interface = make<Interface>();
+    auto interface = make_ref_counted<Interface>();
     interface->module_own_path = this_module;
+    s_resolved_imports.set(this_module, interface);
 
-    NonnullOwnPtrVector<Interface> imports;
+    NonnullRefPtrVector<Interface> imports;
     while (lexer.consume_specific("#import")) {
         consume_whitespace();
         assert_specific('<');
@@ -829,15 +833,12 @@ NonnullOwnPtr<Interface> Parser::parse()
         lexer.ignore();
         auto maybe_interface = resolve_import(path);
         if (maybe_interface.has_value()) {
-            for (auto& entry : maybe_interface.value()->all_imported_paths)
-                s_all_imported_paths.set(entry);
             for (auto& entry : maybe_interface.value()->required_imported_paths)
                 required_imported_paths.set(entry);
             imports.append(maybe_interface.release_value());
         }
         consume_whitespace();
     }
-    interface->all_imported_paths = s_all_imported_paths;
     interface->required_imported_paths = required_imported_paths;
 
     parse_non_interface_entities(true, *interface);
@@ -859,16 +860,16 @@ NonnullOwnPtr<Interface> Parser::parse()
         }
 
         for (auto& typedef_ : import.typedefs)
-            interface->typedefs.set(typedef_.key, move(typedef_.value));
+            interface->typedefs.set(typedef_.key, typedef_.value);
 
         for (auto& mixin : import.mixins) {
-            if (interface->mixins.contains(mixin.key))
+            if (auto it = interface->mixins.find(mixin.key); it != interface->mixins.end() && it->value.ptr() != mixin.value.ptr())
                 report_parsing_error(String::formatted("Mixin '{}' was already defined in {}", mixin.key, mixin.value->module_own_path), filename, input, lexer.tell());
-            interface->mixins.set(mixin.key, move(mixin.value));
+            interface->mixins.set(mixin.key, mixin.value);
         }
 
         for (auto& callback_function : import.callback_functions)
-            interface->callback_functions.set(callback_function.key, move(callback_function.value));
+            interface->callback_functions.set(callback_function.key, callback_function.value);
     }
 
     // Resolve mixins
diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLParser.h b/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLParser.h
index db8148bcc3..de0cd00aaf 100644
--- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLParser.h
+++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLParser.h
@@ -18,7 +18,7 @@ namespace IDL {
 class Parser {
 public:
     Parser(String filename, StringView contents, String import_base_path);
-    NonnullOwnPtr<Interface> parse();
+    NonnullRefPtr<Interface> parse();
 
 private:
     // https://webidl.spec.whatwg.org/#dfn-special-operation
@@ -31,7 +31,7 @@ private:
     void assert_specific(char ch);
     void assert_string(StringView expected);
     void consume_whitespace();
-    Optional<NonnullOwnPtr<Interface>> resolve_import(auto path);
+    Optional<NonnullRefPtr<Interface>> resolve_import(auto path);
 
     HashMap<String, String> parse_extended_attributes();
     void parse_attribute(HashMap<String, String>& extended_attributes, Interface&);
@@ -53,7 +53,7 @@ private:
     NonnullRefPtr<Type> parse_type();
     void parse_constant(Interface&);
 
-    static HashTable<String> s_all_imported_paths;
+    static HashMap<String, NonnullRefPtr<Interface>> s_resolved_imports;
     HashTable<String> required_imported_paths;
     String import_base_path;
     String filename;
diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLTypes.h b/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLTypes.h
index 391891d335..ad8424948e 100644
--- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLTypes.h
+++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLTypes.h
@@ -10,9 +10,8 @@
 #pragma once
 
 #include <AK/HashMap.h>
-#include <AK/NonnullOwnPtrVector.h>
+#include <AK/NonnullRefPtr.h>
 #include <AK/NonnullRefPtrVector.h>
-#include <AK/OwnPtr.h>
 #include <AK/SourceGenerator.h>
 #include <AK/String.h>
 #include <AK/StringBuilder.h>
@@ -165,7 +164,7 @@ static inline size_t get_shortest_function_length(Vector<Function&> const& overl
     return longest_length;
 }
 
-struct Interface {
+struct Interface : public RefCounted<Interface> {
     String name;
     String parent_name;
 
@@ -196,7 +195,7 @@ struct Interface {
     HashMap<String, Dictionary> dictionaries;
     HashMap<String, Enumeration> enumerations;
     HashMap<String, Typedef> typedefs;
-    HashMap<String, NonnullOwnPtr<Interface>> mixins;
+    HashMap<String, NonnullRefPtr<Interface>> mixins;
     HashMap<String, CallbackFunction> callback_functions;
 
     // Added for convenience after parsing
@@ -209,9 +208,8 @@ struct Interface {
     HashMap<String, HashTable<String>> included_mixins;
 
     String module_own_path;
-    HashTable<String> all_imported_paths;
     HashTable<String> required_imported_paths;
-    NonnullOwnPtrVector<Interface> imported_modules;
+    NonnullRefPtrVector<Interface> imported_modules;
 
     HashMap<String, Vector<Function&>> overload_sets;
     HashMap<String, Vector<Function&>> static_overload_sets;