From e16c24bb9580996efab96e84b41237a9e6543e92 Mon Sep 17 00:00:00 2001
From: Itamar <itamar8910@gmail.com>
Date: Fri, 25 Jun 2021 12:39:37 +0300
Subject: [PATCH] HackStudio: Do not create a new LanguageClient unless needed

Previously, whenever Editor::set_document() was called, we destroyed
the previous LanguageClient instance of the editor and created a new
one.

We now check if the language of the existing LanguageClient matches the
new document, and if so we do not create a new LanguageClient instance.

This fixes an issue where doing "goto definition" would crash
HackStudio. This was probably introduced in 44418cb351.

The crash occurred because when doing "goto definition", we called a
AK::Function callback from the LanguageClient, which internally called
Editor::set_document().

Editor::set_document() destroyed the existing LanguageClient, which
cased a VERIFY in Function::clear() to fail because we were trying to
destroy the AK::Function object while executing inside it.
---
 Userland/DevTools/HackStudio/Editor.cpp | 64 ++++++++++++++++---------
 Userland/DevTools/HackStudio/Editor.h   |  2 +
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/Userland/DevTools/HackStudio/Editor.cpp b/Userland/DevTools/HackStudio/Editor.cpp
index eea4a8f01b..69ee6a536f 100644
--- a/Userland/DevTools/HackStudio/Editor.cpp
+++ b/Userland/DevTools/HackStudio/Editor.cpp
@@ -440,33 +440,18 @@ CodeDocument& Editor::code_document()
 
 void Editor::set_document(GUI::TextDocument& doc)
 {
+    if (has_document() && &document() == &doc)
+        return;
+
     VERIFY(doc.is_code_document());
     GUI::TextEditor::set_document(doc);
 
     set_override_cursor(Gfx::StandardCursor::IBeam);
 
-    CodeDocument& code_document = static_cast<CodeDocument&>(doc);
-    switch (code_document.language()) {
-    case Language::Cpp:
-        set_syntax_highlighter(make<Cpp::SyntaxHighlighter>());
-        m_language_client = get_language_client<LanguageClients::Cpp::ServerConnection>(project().root_path());
-        break;
-    case Language::GML:
-        set_syntax_highlighter(make<GUI::GMLSyntaxHighlighter>());
-        break;
-    case Language::JavaScript:
-        set_syntax_highlighter(make<JS::SyntaxHighlighter>());
-        break;
-    case Language::Ini:
-        set_syntax_highlighter(make<GUI::IniSyntaxHighlighter>());
-        break;
-    case Language::Shell:
-        set_syntax_highlighter(make<Shell::SyntaxHighlighter>());
-        m_language_client = get_language_client<LanguageClients::Shell::ServerConnection>(project().root_path());
-        break;
-    default:
-        set_syntax_highlighter({});
-    }
+    auto& code_document = static_cast<CodeDocument&>(doc);
+
+    set_syntax_highlighter_for(code_document);
+    set_language_client_for(code_document);
 
     if (m_language_client) {
         set_autocomplete_provider(make<LanguageServerAidedAutocompleteProvider>(*m_language_client));
@@ -586,4 +571,39 @@ void Editor::set_cursor(const GUI::TextPosition& a_position)
     TextEditor::set_cursor(a_position);
 }
 
+void Editor::set_syntax_highlighter_for(const CodeDocument& document)
+{
+    switch (document.language()) {
+    case Language::Cpp:
+        set_syntax_highlighter(make<Cpp::SyntaxHighlighter>());
+        break;
+    case Language::GML:
+        set_syntax_highlighter(make<GUI::GMLSyntaxHighlighter>());
+        break;
+    case Language::JavaScript:
+        set_syntax_highlighter(make<JS::SyntaxHighlighter>());
+        break;
+    case Language::Ini:
+        set_syntax_highlighter(make<GUI::IniSyntaxHighlighter>());
+        break;
+    case Language::Shell:
+        set_syntax_highlighter(make<Shell::SyntaxHighlighter>());
+        break;
+    default:
+        set_syntax_highlighter({});
+    }
+}
+
+void Editor::set_language_client_for(const CodeDocument& document)
+{
+    if (m_language_client && m_language_client->language() == document.language())
+        return;
+
+    if (document.language() == Language::Cpp)
+        m_language_client = get_language_client<LanguageClients::Cpp::ServerConnection>(project().root_path());
+
+    if (document.language() == Language::Shell)
+        m_language_client = get_language_client<LanguageClients::Shell::ServerConnection>(project().root_path());
+}
+
 }
diff --git a/Userland/DevTools/HackStudio/Editor.h b/Userland/DevTools/HackStudio/Editor.h
index be66ca7839..0b96b6bf5b 100644
--- a/Userland/DevTools/HackStudio/Editor.h
+++ b/Userland/DevTools/HackStudio/Editor.h
@@ -92,6 +92,8 @@ private:
     Optional<AutoCompleteRequestData> get_autocomplete_request_data();
 
     void flush_file_content_to_langauge_server();
+    void set_syntax_highlighter_for(const CodeDocument&);
+    void set_language_client_for(const CodeDocument&);
 
     explicit Editor();