[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Greg Clayton via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb26e1efc424a: [LLDB][GUI] Add Breakpoints window (authored 
by OmarEmaraDev, committed by clayborg).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectRegister.h"
 #include "lldb/Symbol/Block.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/VariableList.h"
@@ -3764,9 +3765,14 @@
TreeItem *_item) {
 return;
   }
-  virtual bool TreeDelegateItemSelected(
-  TreeItem ) = 0; // Return true if we need to update views
+  // This is invoked when a tree item is selected. If true is returned, the
+  // views are updated.
+  virtual bool TreeDelegateItemSelected(TreeItem ) = 0;
   virtual bool TreeDelegateExpandRootByDefault() { return false; }
+  // This is mostly useful for root tree delegates. If false is returned,
+  // drawing will be skipped completely. This is needed, for instance, in
+  // skipping drawing of the threads tree if there is no running process.
+  virtual bool TreeDelegateShouldDraw() { return true; }
 };
 
 typedef std::shared_ptr TreeDelegateSP;
@@ -3956,6 +3962,16 @@
 
   void SetIdentifier(uint64_t identifier) { m_identifier = identifier; }
 
+  const std::string () const { return m_text; }
+
+  void SetText(const char *text) {
+if (text == nullptr) {
+  m_text.clear();
+  return;
+}
+m_text = text;
+  }
+
   void SetMightHaveChildren(bool b) { m_might_have_children = b; }
 
 protected:
@@ -3963,6 +3979,7 @@
   TreeDelegate _delegate;
   void *m_user_data;
   uint64_t m_identifier;
+  std::string m_text;
   int m_row_idx; // Zero based visible row index, -1 if not visible or for the
  // root item
   std::vector m_children;
@@ -3981,21 +3998,6 @@
   int NumVisibleRows() const { return m_max_y - m_min_y; }
 
   bool WindowDelegateDraw(Window , bool force) override {
-ExecutionContext exe_ctx(
-m_debugger.GetCommandInterpreter().GetExecutionContext());
-Process *process = exe_ctx.GetProcessPtr();
-
-bool display_content = false;
-if (process) {
-  StateType state = process->GetState();
-  if (StateIsStoppedState(state, true)) {
-// We are stopped, so it is ok to
-display_content = true;
-  } else if (StateIsRunningState(state)) {
-return true; // Don't do any updating when we are running
-  }
-}
-
 m_min_x = 2;
 m_min_y = 1;
 m_max_x = window.GetWidth() - 1;
@@ -4004,35 +4006,36 @@
 window.Erase();
 window.DrawTitleBox(window.GetName());
 
-if (display_content) {
-  const int num_visible_rows = NumVisibleRows();
-  m_num_rows = 0;
-  m_root.CalculateRowIndexes(m_num_rows);
-  m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
- m_selected_item);
-
-  // If we unexpanded while having something selected our total number of
-  // rows is less than the num visible rows, then make sure we show all the
-  // rows by setting the first visible row accordingly.
-  if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
-m_first_visible_row = 0;
-
-  // Make sure the selected row is always visible
-  if (m_selected_row_idx < m_first_visible_row)
-m_first_visible_row = m_selected_row_idx;
-  else if (m_first_visible_row + num_visible_rows <= m_selected_row_idx)
-m_first_visible_row = m_selected_row_idx - num_visible_rows + 1;
-
-  int row_idx = 0;
-  int num_rows_left = num_visible_rows;
-  m_root.Draw(window, m_first_visible_row, m_selected_row_idx, row_idx,
-  num_rows_left);
-  // Get the selected row
-  m_selected_item = m_root.GetItemForRowIndex(m_selected_row_idx);
-} else {
+if (!m_delegate_sp->TreeDelegateShouldDraw()) {
   m_selected_item = nullptr;
+  return true;
 }
 
+const int num_visible_rows = NumVisibleRows();
+m_num_rows = 0;
+m_root.CalculateRowIndexes(m_num_rows);
+m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
+   m_selected_item);
+
+// If we unexpanded while having something selected our total number of
+// rows is less than the num visible rows, then make sure we show all the
+// rows by setting the first visible row accordingly.
+if (m_first_visible_row > 0 && 

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.

@clayborg This is unrelated to D107761 , so 
it should be safe to commit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Just fix the possible crash when SetText gets a NULL string pointer, and this 
is good to go.

We also need to get the "is_pad(...)" fix in before this can be submitted right?




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3975
+
+  void SetText(const char *text) { m_text = text; }
+

This will crash if you pass in NULL. Test "text" before assignment and call 
m_text.clear() if it is NULL


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.

@jingham I see. Thanks!

Here is how it looks now:

F18450643: 20210809-130123.png 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev updated this revision to Diff 365145.
OmarEmaraDev added a comment.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

- Address review


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectRegister.h"
 #include "lldb/Symbol/Block.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/VariableList.h"
@@ -3772,9 +3773,14 @@
TreeItem *_item) {
 return;
   }
-  virtual bool TreeDelegateItemSelected(
-  TreeItem ) = 0; // Return true if we need to update views
+  // This is invoked when a tree item is selected. If true is returned, the
+  // views are updated.
+  virtual bool TreeDelegateItemSelected(TreeItem ) = 0;
   virtual bool TreeDelegateExpandRootByDefault() { return false; }
+  // This is mostly useful for root tree delegates. If false is returned,
+  // drawing will be skipped completely. This is needed, for instance, in
+  // skipping drawing of the threads tree if there is no running process.
+  virtual bool TreeDelegateShouldDraw() { return true; }
 };
 
 typedef std::shared_ptr TreeDelegateSP;
@@ -3964,6 +3970,10 @@
 
   void SetIdentifier(uint64_t identifier) { m_identifier = identifier; }
 
+  const std::string () const { return m_text; }
+
+  void SetText(const char *text) { m_text = text; }
+
   void SetMightHaveChildren(bool b) { m_might_have_children = b; }
 
 protected:
@@ -3971,6 +3981,7 @@
   TreeDelegate _delegate;
   void *m_user_data;
   uint64_t m_identifier;
+  std::string m_text;
   int m_row_idx; // Zero based visible row index, -1 if not visible or for the
  // root item
   std::vector m_children;
@@ -3989,21 +4000,6 @@
   int NumVisibleRows() const { return m_max_y - m_min_y; }
 
   bool WindowDelegateDraw(Window , bool force) override {
-ExecutionContext exe_ctx(
-m_debugger.GetCommandInterpreter().GetExecutionContext());
-Process *process = exe_ctx.GetProcessPtr();
-
-bool display_content = false;
-if (process) {
-  StateType state = process->GetState();
-  if (StateIsStoppedState(state, true)) {
-// We are stopped, so it is ok to
-display_content = true;
-  } else if (StateIsRunningState(state)) {
-return true; // Don't do any updating when we are running
-  }
-}
-
 m_min_x = 2;
 m_min_y = 1;
 m_max_x = window.GetWidth() - 1;
@@ -4012,35 +4008,36 @@
 window.Erase();
 window.DrawTitleBox(window.GetName());
 
-if (display_content) {
-  const int num_visible_rows = NumVisibleRows();
-  m_num_rows = 0;
-  m_root.CalculateRowIndexes(m_num_rows);
-  m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
- m_selected_item);
-
-  // If we unexpanded while having something selected our total number of
-  // rows is less than the num visible rows, then make sure we show all the
-  // rows by setting the first visible row accordingly.
-  if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
-m_first_visible_row = 0;
-
-  // Make sure the selected row is always visible
-  if (m_selected_row_idx < m_first_visible_row)
-m_first_visible_row = m_selected_row_idx;
-  else if (m_first_visible_row + num_visible_rows <= m_selected_row_idx)
-m_first_visible_row = m_selected_row_idx - num_visible_rows + 1;
-
-  int row_idx = 0;
-  int num_rows_left = num_visible_rows;
-  m_root.Draw(window, m_first_visible_row, m_selected_row_idx, row_idx,
-  num_rows_left);
-  // Get the selected row
-  m_selected_item = m_root.GetItemForRowIndex(m_selected_row_idx);
-} else {
+if (!m_delegate_sp->TreeDelegateShouldDraw()) {
   m_selected_item = nullptr;
+  return true;
 }
 
+const int num_visible_rows = NumVisibleRows();
+m_num_rows = 0;
+m_root.CalculateRowIndexes(m_num_rows);
+m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
+   m_selected_item);
+
+// If we unexpanded while having something selected our total number of
+// rows is less than the num visible rows, then make sure we show all the
+// rows by setting the first visible row accordingly.
+if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
+  m_first_visible_row = 0;
+
+// Make sure the selected row is always visible
+if (m_selected_row_idx < 

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I like the new UI! I think you are right in that we don't need the address on 
the breakpoint location line if we can expand it and see it in the location 
children, so lets remove it from there. Other than that it looks good. The only 
other suggestion I would have is if we should display "1.1" and "1.2" for the 
breakpoint locations. This will match the output of "breakpoint list" a bit 
better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D107386#2932037 , @OmarEmaraDev 
wrote:

> @clayborg With you suggestions it currently looks like this. The concern I 
> have is that lines are now 36 characters longer, and some important 
> information are now truncated. The breakpoint and breakpoint locations IDs 
> seems to be just the index of the objects in the tree, are they not? In this 
> case, they seem redundant to me.

The breakpoint ID & loc ID are how you would refer to the breakpoint or 
location in any console commands, and also to go from the breakpoint ID 
displayed in a stop notification back to the entry in the breakpoints list.  
Those end up being fairly common operations.

You also can't just count nodes to figure out the breakpoint ID from the tree, 
since we don't fill in holes in the numbering when you delete a breakpoint.

I don't have a strong opinion about the addresses, but I think you do need the 
breakpoint & location ID's in the UI somewhere.

> Similarly, the address takes a lot of space and is included in the next level 
> of details, so it might not be as important to include in this level either.
>
> F18404076: 20210806-225259.png 
>
> I am having some difficulties adding the last level of details. My initial 
> approach was to add and compute a StringList containing all the details to 
> the BreakpointLocationTreeDelegate. Then I added a  StringTreeDelegate that 
> have the string from the string list in its user data which  it then simply 
> draws. The problem, I realized, is that the BreakpointLocationTreeDelegate is 
> shared between all Breakpoint Location items, so the list is overwritten by 
> different elements. What I did was instance multiple 
> BreakpointLocationTreeDelegate for each item, but that tuned more involved 
> that I thought, so I am not sure what the best approach would be now. What do 
> you think?




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.

@clayborg With you suggestions it currently looks like this. The concern I have 
is that lines are now 36 characters longer, and some important information are 
now truncated. The breakpoint and breakpoint locations IDs seems to be just the 
index of the objects in the tree, are they not? In this case, they seem 
redundant to me. Similarly, the address takes a lot of space and is included in 
the next level of details, so it might not be as important to include in this 
level either.

F18404076: 20210806-225259.png 

I am having some difficulties adding the last level of details. My initial 
approach was to add and compute a StringList containing all the details to the 
BreakpointLocationTreeDelegate. Then I added a  StringTreeDelegate that have 
the string from the string list in its user data which  it then simply draws. 
The problem, I realized, is that the BreakpointLocationTreeDelegate is shared 
between all Breakpoint Location items, so the list is overwritten by different 
elements. What I did was instance multiple BreakpointLocationTreeDelegate for 
each item, but that tuned more involved that I thought, so I am not sure what 
the best approach would be now. What do you think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D107386#2928910 , @OmarEmaraDev 
wrote:

> By the way, it would be nice to have D107182 
>  commited because it is needed by the 
> search form.

Just committed it!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:4331-4333
+if (StateIsRunningState(process->GetState())) {
+  return false;
+}

Remove {} for single statement if statements per LLVM guidelines



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:4448
+Process *process = m_debugger.GetSelectedTarget()->GetProcessSP().get();
+symbol_context.DumpStopContext(, process, address,
+   false, /* Show full paths. */

I would show the address first, then the dump of the symbol context. You can 
ask the address from the location to dump itself as a load address and fallback 
to module + file address. Check out the Address::Dump function:

```
  bool Address::Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle 
style, DumpStyle fallback_style, uint32_t addr_byte_size) const;
```

To get the raw address first in the string, you can first call the dump with:

```
// Emit the breakpoint location ID
stream.Format("{0} ", breakpoint_location->GetID())
// Emit the the address as a load address if the process is running, or 
fallback to [] format if no process is running
address.Dump(, process,  DumpStyleLoadAddress, 
DumpStyleModuleWithFileAddress, process->GetAddressByteSize());
```

Then you can add a space and then call this again with:

```
stream.PutChar(' ');
// Dump the resolved address description
address.Dump(, process,  DumpStyleResolvedDescription, DumpStyleInvalid, 
process->GetAddressByteSize());
```

I believe that "DumpStyleResolvedDescription" will dump something verify 
similar to what you are dumping here with the 
"symbol_context.DumpStopContext(...)" call.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:4454
+   true /* Show function name. */);
+window.PutCStringTruncated(1, stream.GetString().str().c_str());
+  }

Breakpoint locations have integer IDs. We should show these as the first item 
in the string.

Take a look at the output of "breakpoint list" or "breakpoint list --verbose" 
for ideas on how to format the string.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:4457
+
+  void TreeDelegateGenerateChildren(TreeItem ) override {}
+

We could allow locations to be expanded and then show the details of the 
resolved address like in the "breakpoint list --verbose" output:
```
(lldb) breakpoint list --verbose
Current breakpoints:
1: name = 'main'
1.1: 
  module = /Users/gclayton/Documents/src/args/build/a.out
  compile unit = main.cpp
  function = main
  location = /Users/gclayton/Documents/src/args/main.cpp:5:3
  address = a.out[0x00013f3a]
  resolved = false
  hardware = false
  hit count = 0   
```
We could make a child for each of the lines in this output. I believe the about 
output is from calling Address::Dump(...) with a DumpStyle of 
"DumpStyleDetailedSymbolContext" (see above details on how to call 
Address::Dump(...)). You can dump this to a "StringStream" and then get the 
std::string back from it and split the string using newlines if you like that 
idea. I like the idea of getting full details on a breakpoint location by being 
able to expand it.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:4481
+BreakpointSP breakpoint = GetBreakpoint(item);
+StreamString stream;
+breakpoint->GetResolverDescription();

Breakpoints have integer IDs. We should show these as the first item in the 
string. Not sure on the formatting of this number, maybe just the raw number 
followed by a space or possibly with square brackets and a space.

Take a look at the output of "breakpoint list" or "breakpoint list --verbose" 
for ideas on how to format the string.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:6635
 view_menu_sp->AddSubmenu(
 MenuSP(new Menu("Backtrace", nullptr, 'b',
 ApplicationDelegate::eMenuID_ViewBacktrace)));

We would switch this one to 't' for backTrace and then let the breakpoint view 
take the 'b' key?



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:6646
+view_menu_sp->AddSubmenu(
+MenuSP(new Menu("Breakpoints", nullptr, 'k',
+ApplicationDelegate::eMenuID_ViewBreakpoints)));

switch to 'b' and mentioned above?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.

@clayborg What do you think? Should we add another tree level with more details 
or do something else?

By the way, it would be nice to have D107182  
commited because it is needed by the search form.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.
Herald added a subscriber: JDevlieghere.

I faced a bit of difficulty in this patch, so it is not exactly complete. One 
issue is that breakpoint descriptions can be very long, I tried to implemented 
multiline items to workaround this, but that didn't work out and was a waste of 
time. For now, I just tried to keep descriptions as small as possible. One 
thing I want to try now is to simply add another tree level and add all 
necessary information there, similar to verbose breakpoint description.
This is also missing the breakpoint operators, like enabling and disabling 
breakpoints. Since the window doesn't delegate key handing to the items, this 
will have to be implemented first.

F18327498: 20210803-28.png 

F18327497: 20210803-211059.png 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107386/new/

https://reviews.llvm.org/D107386

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev created this revision.
OmarEmaraDev added a reviewer: clayborg.
Herald added a reviewer: teemperor.
OmarEmaraDev requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch adds a breakpoints window that lists all breakpoints and
breakpoints locations. The window is implemented as a tree, where the
first level is the breakpoints and the second level is breakpoints
locations.

The tree delegate was hardcoded to only draw when there is a process,
which is not necessary for breakpoints, so the relevant logic was
abstracted in the TreeDelegateShouldDraw method.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107386

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -3772,9 +3772,14 @@
TreeItem *_item) {
 return;
   }
-  virtual bool TreeDelegateItemSelected(
-  TreeItem ) = 0; // Return true if we need to update views
+  // This is invoked when a tree item is selected. If true is returned, the
+  // views are updated.
+  virtual bool TreeDelegateItemSelected(TreeItem ) = 0;
   virtual bool TreeDelegateExpandRootByDefault() { return false; }
+  // This is mostly useful for root tree delegates. If false is returned,
+  // drawing will be skipped completely. This is needed, for instance, in
+  // skipping drawing of the threads tree if there is no running process.
+  virtual bool TreeDelegateShouldDraw() { return true; }
 };
 
 typedef std::shared_ptr TreeDelegateSP;
@@ -3989,21 +3994,6 @@
   int NumVisibleRows() const { return m_max_y - m_min_y; }
 
   bool WindowDelegateDraw(Window , bool force) override {
-ExecutionContext exe_ctx(
-m_debugger.GetCommandInterpreter().GetExecutionContext());
-Process *process = exe_ctx.GetProcessPtr();
-
-bool display_content = false;
-if (process) {
-  StateType state = process->GetState();
-  if (StateIsStoppedState(state, true)) {
-// We are stopped, so it is ok to
-display_content = true;
-  } else if (StateIsRunningState(state)) {
-return true; // Don't do any updating when we are running
-  }
-}
-
 m_min_x = 2;
 m_min_y = 1;
 m_max_x = window.GetWidth() - 1;
@@ -4012,35 +4002,36 @@
 window.Erase();
 window.DrawTitleBox(window.GetName());
 
-if (display_content) {
-  const int num_visible_rows = NumVisibleRows();
-  m_num_rows = 0;
-  m_root.CalculateRowIndexes(m_num_rows);
-  m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
- m_selected_item);
-
-  // If we unexpanded while having something selected our total number of
-  // rows is less than the num visible rows, then make sure we show all the
-  // rows by setting the first visible row accordingly.
-  if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
-m_first_visible_row = 0;
-
-  // Make sure the selected row is always visible
-  if (m_selected_row_idx < m_first_visible_row)
-m_first_visible_row = m_selected_row_idx;
-  else if (m_first_visible_row + num_visible_rows <= m_selected_row_idx)
-m_first_visible_row = m_selected_row_idx - num_visible_rows + 1;
-
-  int row_idx = 0;
-  int num_rows_left = num_visible_rows;
-  m_root.Draw(window, m_first_visible_row, m_selected_row_idx, row_idx,
-  num_rows_left);
-  // Get the selected row
-  m_selected_item = m_root.GetItemForRowIndex(m_selected_row_idx);
-} else {
+if (!m_delegate_sp->TreeDelegateShouldDraw()) {
   m_selected_item = nullptr;
+  return true;
 }
 
+const int num_visible_rows = NumVisibleRows();
+m_num_rows = 0;
+m_root.CalculateRowIndexes(m_num_rows);
+m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
+   m_selected_item);
+
+// If we unexpanded while having something selected our total number of
+// rows is less than the num visible rows, then make sure we show all the
+// rows by setting the first visible row accordingly.
+if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
+  m_first_visible_row = 0;
+
+// Make sure the selected row is always visible
+if (m_selected_row_idx < m_first_visible_row)
+  m_first_visible_row = m_selected_row_idx;
+else if (m_first_visible_row + num_visible_rows <= m_selected_row_idx)
+  m_first_visible_row = m_selected_row_idx - num_visible_rows + 1;
+
+int row_idx = 0;
+int num_rows_left = num_visible_rows;
+m_root.Draw(window, m_first_visible_row, m_selected_row_idx, row_idx,
+num_rows_left);
+// Get the selected row
+