Conversation
jruales
commented
Apr 1, 2026
jruales
commented
There was a problem hiding this comment.
Pull request overview
This PR addresses a “Browser View not found” failure mode by preventing BrowserViewModel.setVisible() from calling into IBrowserViewService after the model has already been disposed, avoiding service calls against a no-longer-valid view.
Changes:
- Add a disposal guard in
BrowserViewModel.setVisible()to early-return when the model’s disposable store is already disposed.
| this._visible = visible; // Set optimistically so model is in sync immediately | ||
| if (this._store.isDisposed) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
setVisible updates the model state (this._visible = visible) before checking this._store.isDisposed. If setVisible is invoked after disposal, this still mutates the model even though the service call is skipped, which can leave observers with an inconsistent visible value. Consider returning early before changing _visible, or otherwise ensure disposed instances cannot be observed/used after disposal (e.g. clear listeners / avoid calling setVisible on disposed models).
| this._visible = visible; // Set optimistically so model is in sync immediately | |
| if (this._store.isDisposed) { | |
| return; | |
| } | |
| if (this._store.isDisposed) { | |
| return; | |
| } | |
| this._visible = visible; // Set optimistically so model is in sync immediately |
|
