[lldb-vscode] improve modules request

lldb-vsdode was communicating the list of modules to the IDE with events, which in practice ended up having some drawbacks
- when debugging large targets, the number of these events were easily 10k, which polluted the messages being transmitted, which caused the following: a harder time debugging the messages, a lag after terminated the process because of these messages being processes (this could easily take several seconds). The latter was specially bad, as users were complaining about it even when they didn't check the modules view.
- these events were rarely used, as users only check the modules view when something is wrong and they try to debug things.

After getting some feedback from users, we realized that it's better to not used events but make this simply a request and is triggered by users whenever they needed.

This diff achieves that and does some small clean up in the existing code.

Differential Revision: https://reviews.llvm.org/D94033
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 70f29cd..0962e36 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -113,7 +113,6 @@
         self.initialize_body = None
         self.thread_stop_reasons = {}
         self.breakpoint_events = []
-        self.module_events = {}
         self.sequence = 1
         self.threads = None
         self.recv_thread.start()
@@ -134,8 +133,12 @@
         if command['seq'] != response['request_seq']:
             raise ValueError('seq mismatch in response')
 
-    def get_active_modules(self):
-        return self.module_events
+    def get_modules(self):
+        module_list = self.request_modules()['body']['modules']
+        modules = {}
+        for module in module_list:
+            modules[module['name']] = module
+        return modules
         
     def get_output(self, category, timeout=0.0, clear=True):
         self.output_condition.acquire()
@@ -222,14 +225,6 @@
                 self.breakpoint_events.append(packet)
                 # no need to add 'breakpoint' event packets to our packets list
                 return keepGoing
-            elif event == 'module':
-                reason = body['reason']
-                if (reason == 'new' or reason == 'changed'):
-                    self.module_events[body['module']['name']] = body['module']
-                elif reason == 'removed':
-                    if body['module']['name'] in self.module_events:
-                        self.module_events.pop(body['module']['name'])
-                return keepGoing
 
         elif packet_type == 'response':
             if packet['command'] == 'disconnect':
@@ -782,10 +777,10 @@
         }
         return self.send_recv(command_dict)
 
-    def request_getCompileUnits(self, moduleId):
+    def request_compileUnits(self, moduleId):
         args_dict = {'moduleId': moduleId}
         command_dict = {
-            'command': 'getCompileUnits',
+            'command': 'compileUnits',
             'type': 'request',
             'arguments': args_dict
         }
@@ -804,6 +799,12 @@
         }
         return self.send_recv(command_dict)
 
+    def request_modules(self):
+        return self.send_recv({
+            'command': 'modules',
+            'type': 'request'
+        })
+
     def request_stackTrace(self, threadId=None, startFrame=None, levels=None,
                            dump=False):
         if threadId is None:
diff --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
index 7fa5f7d..40ac7c4 100644
--- a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -24,7 +24,7 @@
         breakpoint_ids = self.set_function_breakpoints(functions)
         self.assertEquals(len(breakpoint_ids), len(functions), 'expect one breakpoint')
         self.continue_to_breakpoints(breakpoint_ids)
-        active_modules = self.vscode.get_active_modules()
+        active_modules = self.vscode.get_modules()
         program_module = active_modules[program_basename]
         self.assertIn(program_basename, active_modules, '%s module is in active modules' % (program_basename))
         self.assertIn('name', program_module, 'make sure name is in module')
@@ -36,7 +36,7 @@
         self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" "%s"' % (program, symbols_path)))
 
         def checkSymbolsLoadedWithSize():
-            active_modules = self.vscode.get_active_modules()
+            active_modules = self.vscode.get_modules()
             program_module = active_modules[program_basename]
             self.assertIn('symbolFilePath', program_module)
             self.assertIn(symbols_path, program_module['symbolFilePath'])
@@ -45,17 +45,15 @@
                 
         if expect_debug_info_size:
             self.waitUntil(checkSymbolsLoadedWithSize)
-        active_modules = self.vscode.get_active_modules()
+        active_modules = self.vscode.get_modules()
         program_module = active_modules[program_basename]
         self.assertEqual(program_basename, program_module['name'])
         self.assertEqual(program, program_module['path'])
         self.assertIn('addressRange', program_module)
 
     @skipIfWindows
-    @skipUnlessDarwin
     @skipIfRemote  
-    #TODO: Update the Makefile so that this test runs on Linux
-    def test_module_event(self):
+    def test_modules(self):
         '''
             Mac or linux.
 
@@ -64,27 +62,21 @@
             sections are in .o files.
 
             On other platforms, we expect a.out to have debug info, so we will expect a size.
-            expect_debug_info_size = platform.system() != 'Darwin'
-            return self.run_test("a.out", expect_debug_info_size)
         '''
-        expect_debug_info_size = platform.system() != 'Darwin'
-        return self.run_test("a.out", expect_debug_info_size)
+        return self.run_test("a.out", expect_debug_info_size=platform.system() != 'Darwin')
 
-    @skipIfWindows
     @skipUnlessDarwin
     @skipIfRemote    
-    def test_module_event_dsym(self):
+    def test_modules_dsym(self):
         '''
             Darwin only test with dSYM file.
 
             On mac, if we load a.out.dSYM as our symbol file, we will have debug symbols and we
             will have DWARF sections added to the module, so we will expect a size.
-            return self.run_test("a.out.dSYM", True)
         '''
-        return self.run_test("a.out.dSYM", True)
+        return self.run_test("a.out.dSYM", expect_debug_info_size=True)
 
     @skipIfWindows
-    @skipUnlessDarwin
     @skipIfRemote
     def test_compile_units(self):
         program = self.getBuildArtifact("a.out")
@@ -95,11 +87,10 @@
         lines = [breakpoint1_line]
         breakpoint_ids = self.set_source_breakpoints(source, lines)
         self.continue_to_breakpoints(breakpoint_ids)
-        moduleId = self.vscode.get_active_modules()['a.out']['id']
-        response = self.vscode.request_getCompileUnits(moduleId)
+        moduleId = self.vscode.get_modules()['a.out']['id']
+        response = self.vscode.request_compileUnits(moduleId)
         self.assertTrue(response['body'])
         self.assertTrue(len(response['body']['compileUnits']) == 1,
                         'Only one source file should exist')
         self.assertTrue(response['body']['compileUnits'][0]['compileUnitPath'] == main_source_path,
                         'Real path to main.cpp matches')
-
diff --git a/lldb/tools/lldb-vscode/VSCode.cpp b/lldb/tools/lldb-vscode/VSCode.cpp
index 9450cdf..e9fdc17 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -355,11 +355,6 @@
         lldb::SBTarget::eBroadcastBitBreakpointChanged);
     listener.StartListeningForEvents(this->broadcaster,
                                      eBroadcastBitStopEventThread);
-    listener.StartListeningForEvents(
-        this->target.GetBroadcaster(),
-        lldb::SBTarget::eBroadcastBitModulesLoaded |
-            lldb::SBTarget::eBroadcastBitModulesUnloaded |
-            lldb::SBTarget::eBroadcastBitSymbolsLoaded);
   }
 }
 
diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index b648294..dcb5cfe 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -440,30 +440,6 @@
             g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
           }
         }
-      } else if (lldb::SBTarget::EventIsTargetEvent(event)) {
-        if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded ||
-            event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded ||
-            event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
-          int num_modules = lldb::SBTarget::GetNumModulesFromEvent(event);
-          for (int i = 0; i < num_modules; i++) {
-            auto module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
-            auto module_event = CreateEventObject("module");
-            llvm::json::Value module_value = CreateModule(module);
-            llvm::json::Object body;
-            if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) {
-              body.try_emplace("reason", "new");
-            } else if (event_mask &
-                       lldb::SBTarget::eBroadcastBitModulesUnloaded) {
-              body.try_emplace("reason", "removed");
-            } else if (event_mask &
-                       lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
-              body.try_emplace("reason", "changed");
-            }
-            body.try_emplace("module", module_value);
-            module_event.try_emplace("body", std::move(body));
-            g_vsc.SendJSON(llvm::json::Value(std::move(module_event)));
-          }
-        }
       } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
         if (event_mask & eBroadcastBitStopEventThread) {
           done = true;
@@ -1196,26 +1172,26 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
-// "getCompileUnitsRequest": {
+// "compileUnitsRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
 //     "type": "object",
 //     "description": "Compile Unit request; value of command field is
-//                     'getCompileUnits'.",
+//                     'compileUnits'.",
 //     "properties": {
 //       "command": {
 //         "type": "string",
-//         "enum": [ "getCompileUnits" ]
+//         "enum": [ "compileUnits" ]
 //       },
 //       "arguments": {
-//         "$ref": "#/definitions/getCompileUnitRequestArguments"
+//         "$ref": "#/definitions/compileUnitRequestArguments"
 //       }
 //     },
 //     "required": [ "command", "arguments" ]
 //   }]
 // },
-// "getCompileUnitsRequestArguments": {
+// "compileUnitsRequestArguments": {
 //   "type": "object",
-//   "description": "Arguments for 'getCompileUnits' request.",
+//   "description": "Arguments for 'compileUnits' request.",
 //   "properties": {
 //     "moduleId": {
 //       "type": "string",
@@ -1224,23 +1200,21 @@
 //   },
 //   "required": [ "moduleId" ]
 // },
-// "getCompileUnitsResponse": {
+// "compileUnitsResponse": {
 //   "allOf": [ { "$ref": "#/definitions/Response" }, {
 //     "type": "object",
-//     "description": "Response to 'getCompileUnits' request.",
+//     "description": "Response to 'compileUnits' request.",
 //     "properties": {
 //       "body": {
-//         "description": "Response to 'getCompileUnits' request. Array of
+//         "description": "Response to 'compileUnits' request. Array of
 //                         paths of compile units."
 //       }
 //     }
 //   }]
 // }
-
-void request_getCompileUnits(const llvm::json::Object &request) {
+void request_compileUnits(const llvm::json::Object &request) {
   llvm::json::Object response;
   FillResponse(request, response);
-  lldb::SBProcess process = g_vsc.target.GetProcess();
   llvm::json::Object body;
   llvm::json::Array units;
   auto arguments = request.getObject("arguments");
@@ -1262,6 +1236,48 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+// "modulesRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+//     "type": "object",
+//     "description": "Modules request; value of command field is
+//                     'modules'.",
+//     "properties": {
+//       "command": {
+//         "type": "string",
+//         "enum": [ "modules" ]
+//       },
+//     },
+//     "required": [ "command" ]
+//   }]
+// },
+// "modulesResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+//     "type": "object",
+//     "description": "Response to 'modules' request.",
+//     "properties": {
+//       "body": {
+//         "description": "Response to 'modules' request. Array of
+//                         module objects."
+//       }
+//     }
+//   }]
+// }
+void request_modules(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+
+  llvm::json::Array modules;
+  for (size_t i = 0; i < g_vsc.target.GetNumModules(); i++) {
+    lldb::SBModule module = g_vsc.target.GetModuleAtIndex(i);
+    modules.emplace_back(CreateModule(module));
+  }
+
+  llvm::json::Object body;
+  body.try_emplace("modules", std::move(modules));
+  response.try_emplace("body", std::move(body));
+  g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+}
+
 // "InitializeRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
 //     "type": "object",
@@ -2901,7 +2917,6 @@
   g_vsc.RegisterRequestCallback("disconnect", request_disconnect);
   g_vsc.RegisterRequestCallback("evaluate", request_evaluate);
   g_vsc.RegisterRequestCallback("exceptionInfo", request_exceptionInfo);
-  g_vsc.RegisterRequestCallback("getCompileUnits", request_getCompileUnits);
   g_vsc.RegisterRequestCallback("initialize", request_initialize);
   g_vsc.RegisterRequestCallback("launch", request_launch);
   g_vsc.RegisterRequestCallback("next", request_next);
@@ -2919,6 +2934,9 @@
   g_vsc.RegisterRequestCallback("stepOut", request_stepOut);
   g_vsc.RegisterRequestCallback("threads", request_threads);
   g_vsc.RegisterRequestCallback("variables", request_variables);
+  // Custom requests
+  g_vsc.RegisterRequestCallback("compileUnits", request_compileUnits);
+  g_vsc.RegisterRequestCallback("modules", request_modules);
   // Testing requests
   g_vsc.RegisterRequestCallback("_testGetTargetBreakpoints",
                                 request__testGetTargetBreakpoints);