[LMI] Move highlightInfo storage from View to Controller
This CL changes where highlightInfo is stored and
how it gets passed from the Controller to the UI components.
Previously, highlightInfo was cached inside of
the LinearMemoryInspectorView instances. This required
a clunky data-flow between the Controller and the
InspectorView. Instead, this CL moves the storage
of highlightInfo to the Controller and exposes a getter
function for the UI components to use.
We store highlightInfos in a Map
with bufferIds as keys. This will be easy to extend to
support storing multiple highlights. To make access
possible from the InspectorView, we pass it its bufferId.
Bug: 1336568
Change-Id: Ib6387a174701e9a720b5602100f0e7d5454f1ac5
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3784598
Commit-Queue: Kim-Anh Tran <[email protected]>
Auto-Submit: Michal Pitr <[email protected]>
Reviewed-by: Kim-Anh Tran <[email protected]>
diff --git a/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorController.ts b/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorController.ts
index 8c7fdc2..89c87a7 100644
--- a/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorController.ts
+++ b/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorController.ts
@@ -111,6 +111,7 @@
export class LinearMemoryInspectorController extends SDK.TargetManager.SDKModelObserver<SDK.RuntimeModel.RuntimeModel> {
#paneInstance = LinearMemoryInspectorPaneImpl.instance();
#bufferIdToRemoteObject: Map<string, SDK.RemoteObject.RemoteObject> = new Map();
+ #bufferIdToHighlightInfo: Map<string, HighlightInfo> = new Map();
#settings: Common.Settings.Setting<SerializableSettings>;
private constructor() {
@@ -178,6 +179,18 @@
};
}
+ getHighlightInfo(bufferId: string): HighlightInfo|undefined {
+ return this.#bufferIdToHighlightInfo.get(bufferId);
+ }
+
+ #setHighlightInfo(bufferId: string, highlightInfo: HighlightInfo): void {
+ this.#bufferIdToHighlightInfo.set(bufferId, highlightInfo);
+ }
+
+ #resetHighlightInfo(bufferId: string): void {
+ this.#bufferIdToHighlightInfo.delete(bufferId);
+ }
+
static async retrieveDWARFMemoryObjectAndAddress(obj: SDK.RemoteObject.RemoteObject):
Promise<{obj: SDK.RemoteObject.RemoteObject, address: number}|undefined> {
if (obj instanceof Bindings.DebuggerLanguagePlugins.ExtensionRemoteObject) {
@@ -253,8 +266,6 @@
memoryObj = response.obj;
}
- const highlightInfo = this.#extractHighlightInfo(obj, memoryAddress);
-
if (memoryAddress !== undefined) {
Host.userMetrics.linearMemoryInspectorTarget(
Host.UserMetrics.LinearMemoryInspectorTarget.DWARFInspectableAddress);
@@ -277,9 +288,14 @@
}
const memoryProperty = internalProperties?.find(({name}) => name === '[[WebAssemblyMemory]]');
const memory = memoryProperty?.value;
-
+ const highlightInfo = LinearMemoryInspectorController.extractHighlightInfo(obj, memoryAddress);
+ if (highlightInfo !== undefined) {
+ this.#setHighlightInfo(id, highlightInfo);
+ } else {
+ this.#resetHighlightInfo(id);
+ }
if (this.#bufferIdToRemoteObject.has(id)) {
- this.#paneInstance.reveal(id, memoryAddress, highlightInfo);
+ this.#paneInstance.reveal(id, memoryAddress);
void UI.ViewManager.ViewManager.instance().showView('linear-memory-inspector');
return;
}
@@ -288,11 +304,11 @@
this.#bufferIdToRemoteObject.set(id, buffer.object());
const arrayBufferWrapper = new RemoteArrayBufferWrapper(buffer);
- this.#paneInstance.create(id, title, arrayBufferWrapper, memoryAddress, highlightInfo);
+ this.#paneInstance.create(id, title, arrayBufferWrapper, memoryAddress);
void UI.ViewManager.ViewManager.instance().showView('linear-memory-inspector');
}
- #extractHighlightInfo(obj: SDK.RemoteObject.RemoteObject, memoryAddress?: number): HighlightInfo|undefined {
+ static extractHighlightInfo(obj: SDK.RemoteObject.RemoteObject, memoryAddress?: number): HighlightInfo|undefined {
let highlightInfo;
if (obj instanceof Bindings.DebuggerLanguagePlugins.ValueNode) {
try {
@@ -311,6 +327,7 @@
for (const [bufferId, remoteObject] of this.#bufferIdToRemoteObject) {
if (model === remoteObject.runtimeModel()) {
this.#bufferIdToRemoteObject.delete(bufferId);
+ this.#resetHighlightInfo(bufferId);
this.#paneInstance.close(bufferId);
}
}
@@ -320,7 +337,7 @@
const debuggerModel = event.data;
for (const [bufferId, remoteObject] of this.#bufferIdToRemoteObject) {
if (debuggerModel.runtimeModel() === remoteObject.runtimeModel()) {
- this.#paneInstance.resetHighlightInfo(bufferId);
+ this.#resetHighlightInfo(bufferId);
this.#paneInstance.refreshView(bufferId);
}
}
@@ -336,5 +353,6 @@
remoteObj.release();
}
this.#bufferIdToRemoteObject.delete(bufferId);
+ this.#resetHighlightInfo(bufferId);
}
}
diff --git a/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorPane.ts b/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorPane.ts
index cc1f24b..5d6562a 100644
--- a/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorPane.ts
+++ b/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorPane.ts
@@ -84,14 +84,13 @@
getViewForTabId(tabId: string): LinearMemoryInspectorView {
const view = this.#tabIdToInspectorView.get(tabId);
if (!view) {
- throw new Error(`No linear memory inspector view for given tab id: ${tabId}`);
+ throw new Error(`No linear memory inspector view for the given tab id: ${tabId}`);
}
return view;
}
- create(tabId: string, title: string, arrayWrapper: LazyUint8Array, address?: number, highlightInfo?: HighlightInfo):
- void {
- const inspectorView = new LinearMemoryInspectorView(arrayWrapper, address, highlightInfo);
+ create(tabId: string, title: string, arrayWrapper: LazyUint8Array, address?: number): void {
+ const inspectorView = new LinearMemoryInspectorView(arrayWrapper, address, tabId);
this.#tabIdToInspectorView.set(tabId, inspectorView);
this.#tabbedPane.appendTab(tabId, title, inspectorView, undefined, false, true);
this.#tabbedPane.selectTab(tabId);
@@ -101,15 +100,12 @@
this.#tabbedPane.closeTab(tabId, false);
}
- reveal(tabId: string, address?: number, highlightInfo?: HighlightInfo): void {
+ reveal(tabId: string, address?: number): void {
const view = this.getViewForTabId(tabId);
if (address !== undefined) {
view.updateAddress(address);
}
- if (highlightInfo !== undefined) {
- view.updateHighlightInfo(highlightInfo);
- }
this.refreshView(tabId);
this.#tabbedPane.selectTab(tabId);
}
@@ -119,11 +115,6 @@
view.refreshData();
}
- resetHighlightInfo(tabId: string): void {
- const view = this.getViewForTabId(tabId);
- view.updateHighlightInfo(undefined);
- }
-
#tabClosed(event: Common.EventTarget.EventTargetEvent<UI.TabbedPane.EventData>): void {
const {tabId} = event.data;
this.#tabIdToInspectorView.delete(tabId);
@@ -142,10 +133,10 @@
class LinearMemoryInspectorView extends UI.Widget.VBox {
#memoryWrapper: LazyUint8Array;
#address: number;
- #highlightInfo?: HighlightInfo;
+ #tabId: string;
#inspector: LinearMemoryInspector;
firstTimeOpen: boolean;
- constructor(memoryWrapper: LazyUint8Array, address: number|undefined = 0, highlightInfo?: HighlightInfo) {
+ constructor(memoryWrapper: LazyUint8Array, address: number|undefined = 0, tabId: string) {
super(false);
if (address < 0 || address >= memoryWrapper.length()) {
@@ -154,7 +145,7 @@
this.#memoryWrapper = memoryWrapper;
this.#address = address;
- this.#highlightInfo = highlightInfo;
+ this.#tabId = tabId;
this.#inspector = new LinearMemoryInspector();
this.#inspector.addEventListener('memoryrequest', (event: MemoryRequestEvent) => {
this.#memoryRequested(event);
@@ -186,18 +177,6 @@
this.#address = address;
}
- updateHighlightInfo(highlightInfo?: HighlightInfo): void {
- if (highlightInfo !== undefined) {
- if (highlightInfo.startAddress < 0 || highlightInfo.startAddress >= this.#memoryWrapper.length()) {
- throw new Error('Highlight info start address is out of bounds.');
- }
- if (highlightInfo.size < 0) {
- throw new Error('Highlight size cannot be negative.');
- }
- }
- this.#highlightInfo = highlightInfo;
- }
-
refreshData(): void {
void LinearMemoryInspectorController.getMemoryForAddress(this.#memoryWrapper, this.#address).then(({
memory,
@@ -221,7 +200,7 @@
valueTypes,
valueTypeModes,
endianness,
- highlightInfo: this.#highlightInfo,
+ highlightInfo: this.#getHighlightInfo(),
};
});
}
@@ -238,8 +217,21 @@
address: address,
memoryOffset: start,
outerMemoryLength: this.#memoryWrapper.length(),
- highlightInfo: this.#highlightInfo,
+ highlightInfo: this.#getHighlightInfo(),
};
});
}
+
+ #getHighlightInfo(): HighlightInfo|undefined {
+ const highlightInfo = LinearMemoryInspectorController.instance().getHighlightInfo(this.#tabId);
+ if (highlightInfo !== undefined) {
+ if (highlightInfo.startAddress < 0 || highlightInfo.startAddress >= this.#memoryWrapper.length()) {
+ throw new Error('Highlight info start address is out of bounds.');
+ }
+ if (highlightInfo.size <= 0) {
+ throw new Error('Highlight size must be a positive number.');
+ }
+ }
+ return highlightInfo;
+ }
}
diff --git a/test/unittests/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorPane_test.ts b/test/unittests/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorPane_test.ts
index c5b95b7..2ec1e61 100644
--- a/test/unittests/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorPane_test.ts
+++ b/test/unittests/front_end/ui/components/linear_memory_inspector/LinearMemoryInspectorPane_test.ts
@@ -52,43 +52,4 @@
'devtools-linear-memory-inspector-inspector');
assertElement(inspector, LinearMemoryInspector.LinearMemoryInspector.LinearMemoryInspector);
});
-
- it('triggers view.updateHighlightInfo() when resetHighlightInfo() called', () => {
- const instance = LinearMemoryInspector.LinearMemoryInspectorPane.LinearMemoryInspectorPaneImpl.instance();
- const arrayWrapper = new Uint8Wrapper(createArray());
- const tabId = 'tabId';
- const title = 'Test Title';
-
- const highlightInfo = {
- startAddress: 2,
- size: 10,
- };
- instance.create(tabId, title, arrayWrapper, 10, highlightInfo);
- const view = instance.getViewForTabId(tabId);
- const spy = sinon.spy(view, 'updateHighlightInfo');
- instance.resetHighlightInfo(tabId);
- assert.isTrue(spy.calledOnce);
- });
-
- it('triggers view.refreshData() when reveal() called with highlightInfo set', () => {
- const instance = LinearMemoryInspector.LinearMemoryInspectorPane.LinearMemoryInspectorPaneImpl.instance();
- const arrayWrapper = new Uint8Wrapper(createArray());
- const tabId = 'tabId';
- const title = 'Test Title';
-
- const oldHighlightInfo = {
- startAddress: 2,
- size: 10,
- };
- const newHighlightInfo = {
- startAddress: 4,
- size: 4,
- };
-
- instance.create(tabId, title, arrayWrapper, 10, oldHighlightInfo);
- const view = instance.getViewForTabId(tabId);
- const spy = sinon.spy(view, 'refreshData');
- instance.reveal(tabId, 4, newHighlightInfo);
- assert.isTrue(spy.calledOnce);
- });
});
