Skip to content

Commit 13074bd

Browse files
szuendDevtools-frontend LUCI CQ
authored andcommitted
[network] Move RequestBinaryResponseView to different module
This CL renames the "RequestBinaryResponseView" to "StreamingContentHexView" and moves it to "source_frame" components. We want to re-use the linear memory inspector in the BinaryResourceView(Factory) and importing the network panel there would be a layering violation. At the same time, we also can't depend on the linear memory inspector panel directly as well, as that would also be a layering violation. To solve this, this CL re-implements the "LinearMemoryInspectorView" in a way that doesn't have a dependency on the "LinearMemoryInspectorController". In a follow-up CL the "StreamingContentHexView", will replace the "hex" view portion of the "BinaryResourceView(Factory)". The utf-8 and base64 will stay as-is modulo supporting streaming content. [email protected] Bug: 375546679 Change-Id: If2f3ce62e8f72bd29d207b998aba17613d3b8237 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5972706 Reviewed-by: Kim-Anh Tran <[email protected]> Commit-Queue: Simon Zünd <[email protected]>
1 parent ea3fa51 commit 13074bd

File tree

11 files changed

+144
-89
lines changed

11 files changed

+144
-89
lines changed

config/gni/devtools_grd_files.gni

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1515,7 +1515,6 @@ grd_files_debug_sources = [
15151515
"front_end/panels/network/NetworkSearchScope.js",
15161516
"front_end/panels/network/NetworkTimeCalculator.js",
15171517
"front_end/panels/network/NetworkWaterfallColumn.js",
1518-
"front_end/panels/network/RequestBinaryResponseView.js",
15191518
"front_end/panels/network/RequestCookiesView.js",
15201519
"front_end/panels/network/RequestHTMLView.js",
15211520
"front_end/panels/network/RequestInitiatorView.js",
@@ -2407,6 +2406,7 @@ grd_files_debug_sources = [
24072406
"front_end/ui/legacy/components/source_frame/PreviewFactory.js",
24082407
"front_end/ui/legacy/components/source_frame/ResourceSourceFrame.js",
24092408
"front_end/ui/legacy/components/source_frame/SourceFrame.js",
2409+
"front_end/ui/legacy/components/source_frame/StreamingContentHexView.js",
24102410
"front_end/ui/legacy/components/source_frame/XMLView.js",
24112411
"front_end/ui/legacy/components/source_frame/fontView.css.legacy.js",
24122412
"front_end/ui/legacy/components/source_frame/imageView.css.legacy.js",

front_end/panels/linear_memory_inspector/components/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ devtools_entrypoint("bundle") {
5959
":*",
6060
"../:*",
6161
"../../../ui/components/docs/linear_memory_inspector/*",
62+
"../../../ui/legacy/components/source_frame/*",
6263
"../../network:*",
6364
]
6465

front_end/panels/network/BUILD.gn

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ devtools_module("network") {
4949
"NetworkSearchScope.ts",
5050
"NetworkTimeCalculator.ts",
5151
"NetworkWaterfallColumn.ts",
52-
"RequestBinaryResponseView.ts",
5352
"RequestCookiesView.ts",
5453
"RequestHTMLView.ts",
5554
"RequestInitiatorView.ts",
@@ -151,7 +150,6 @@ ts_library("unittests") {
151150
"NetworkOverview.test.ts",
152151
"NetworkPanel.test.ts",
153152
"NetworkSearchScope.test.ts",
154-
"RequestBinaryResponseView.test.ts",
155153
"RequestCookiesView.test.ts",
156154
"RequestHTMLView.test.ts",
157155
"RequestPayloadView.test.ts",

front_end/panels/network/RequestBinaryResponseView.ts

Lines changed: 0 additions & 67 deletions
This file was deleted.

front_end/panels/network/RequestResponseView.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describeWithEnvironment('RequestResponseView', () => {
4242
component.detach();
4343
});
4444

45-
it('shows the RequestBinaryResponseView for binary content', async () => {
45+
it('shows the StreamingContentHexView for binary content', async () => {
4646
const request = SDK.NetworkRequest.NetworkRequest.create(
4747
'requestId' as Protocol.Network.RequestId,
4848
'http://devtools-frontend.test/image.png' as Platform.DevToolsPath.UrlString,
@@ -60,7 +60,7 @@ describeWithEnvironment('RequestResponseView', () => {
6060
component.show(document.body);
6161
const widget = await showPreviewSpy.returnValues[0];
6262

63-
assert.instanceOf(widget, Network.RequestBinaryResponseView.RequestBinaryResponseView);
63+
assert.instanceOf(widget, SourceFrame.StreamingContentHexView.StreamingContentHexView);
6464

6565
await raf();
6666
component.detach();

front_end/panels/network/RequestResponseView.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ import * as SourceFrame from '../../ui/legacy/components/source_frame/source_fra
3737
import * as UI from '../../ui/legacy/legacy.js';
3838
import * as VisualLogging from '../../ui/visual_logging/visual_logging.js';
3939

40-
import {RequestBinaryResponseView} from './RequestBinaryResponseView.js';
41-
4240
const UIStrings = {
4341
/**
4442
*@description Text in Request Response View of the Network panel
@@ -96,7 +94,7 @@ export class RequestResponseView extends UI.Widget.VBox {
9694
// Note: Even though WASM is binary data, the source view will disassemble it and show a text representation.
9795
sourceView = SourceFrame.ResourceSourceFrame.ResourceSourceFrame.createSearchableView(request, mimeType);
9896
} else {
99-
sourceView = new RequestBinaryResponseView(contentData);
97+
sourceView = new SourceFrame.StreamingContentHexView.StreamingContentHexView(contentData);
10098
}
10199

102100
requestToSourceView.set(request, sourceView);

front_end/panels/network/network.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import * as NetworkPanel from './NetworkPanel.js';
4141
import * as NetworkSearchScope from './NetworkSearchScope.js';
4242
import * as NetworkTimeCalculator from './NetworkTimeCalculator.js';
4343
import * as NetworkWaterfallColumn from './NetworkWaterfallColumn.js';
44-
import * as RequestBinaryResponseView from './RequestBinaryResponseView.js';
4544
import * as RequestCookiesView from './RequestCookiesView.js';
4645
import * as RequestHTMLView from './RequestHTMLView.js';
4746
import * as RequestInitiatorView from './RequestInitiatorView.js';
@@ -68,7 +67,6 @@ export {
6867
NetworkSearchScope,
6968
NetworkTimeCalculator,
7069
NetworkWaterfallColumn,
71-
RequestBinaryResponseView,
7270
RequestCookiesView,
7371
RequestHTMLView,
7472
RequestInitiatorView,

front_end/ui/legacy/components/source_frame/BUILD.gn

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ devtools_module("source_frame") {
3131
"PreviewFactory.ts",
3232
"ResourceSourceFrame.ts",
3333
"SourceFrame.ts",
34+
"StreamingContentHexView.ts",
3435
"XMLView.ts",
3536
]
3637

@@ -43,6 +44,7 @@ devtools_module("source_frame") {
4344
"../../../../models/formatter:bundle",
4445
"../../../../models/text_utils:bundle",
4546
"../../../../models/workspace:bundle",
47+
"../../../../panels/linear_memory_inspector/components:bundle",
4648
"../../../../third_party/diff:bundle",
4749
"../../../../ui/components/text_editor:bundle",
4850
"../../../../ui/legacy:bundle",
@@ -92,6 +94,7 @@ ts_library("unittests") {
9294
"BinaryResourceViewFactory.test.ts",
9395
"ResourceSourceFrame.test.ts",
9496
"SourceFrame.test.ts",
97+
"StreamingContentHexView.test.ts",
9598
]
9699

97100
deps = [

front_end/panels/network/RequestBinaryResponseView.test.ts renamed to front_end/ui/legacy/components/source_frame/StreamingContentHexView.test.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1-
// Copyright (c) 2024 The Chromium Authors. All rights reserved.
1+
// Copyright 2024 The Chromium Authors. All rights reserved.
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
import * as TextUtils from '../../models/text_utils/text_utils.js';
6-
import {raf} from '../../testing/DOMHelpers.js';
7-
import {describeWithEnvironment} from '../../testing/EnvironmentHelpers.js';
8-
import * as LinearMemoryInspectorComponents from '../linear_memory_inspector/components/components.js';
5+
import * as TextUtils from '../../../../models/text_utils/text_utils.js';
6+
import * as LinearMemoryInspectorComponents from '../../../../panels/linear_memory_inspector/components/components.js';
7+
import {raf} from '../../../../testing/DOMHelpers.js';
8+
import {describeWithEnvironment} from '../../../../testing/EnvironmentHelpers.js';
99

10-
import * as Network from './network.js';
10+
import * as SourceFrame from './source_frame.js';
1111

12-
describeWithEnvironment('RequestBinaryResponseView', () => {
13-
function getMemoryViewer(view: Network.RequestBinaryResponseView.RequestBinaryResponseView):
12+
describeWithEnvironment('StreamingContentHexView', () => {
13+
function getMemoryViewer(view: SourceFrame.StreamingContentHexView.StreamingContentHexView):
1414
LinearMemoryInspectorComponents.LinearMemoryViewer.LinearMemoryViewer {
1515
const inspector = view.contentElement.firstChild as HTMLElement;
1616
assert.isNotNull(inspector.shadowRoot);
@@ -20,15 +20,15 @@ describeWithEnvironment('RequestBinaryResponseView', () => {
2020
return viewer;
2121
}
2222

23-
function getAllByteCells(view: Network.RequestBinaryResponseView.RequestBinaryResponseView): string {
23+
function getAllByteCells(view: SourceFrame.StreamingContentHexView.StreamingContentHexView): string {
2424
const viewer = getMemoryViewer(view);
2525
assert.isNotNull(viewer.shadowRoot);
2626

2727
const byteCells = [...viewer.shadowRoot.querySelectorAll('.byte-cell')];
2828
return byteCells.map(c => c.textContent).join('');
2929
}
3030

31-
function getAllTextCells(view: Network.RequestBinaryResponseView.RequestBinaryResponseView): string {
31+
function getAllTextCells(view: SourceFrame.StreamingContentHexView.StreamingContentHexView): string {
3232
const viewer = getMemoryViewer(view);
3333
assert.isNotNull(viewer.shadowRoot);
3434

@@ -39,7 +39,7 @@ describeWithEnvironment('RequestBinaryResponseView', () => {
3939
it('shows the initial content of a StreamingContentData', async () => {
4040
const streamingContentData = TextUtils.StreamingContentData.StreamingContentData.from(
4141
new TextUtils.ContentData.ContentData(window.btoa('abc'), /* isBase64 */ true, 'application/octet-stream'));
42-
const view = new Network.RequestBinaryResponseView.RequestBinaryResponseView(streamingContentData);
42+
const view = new SourceFrame.StreamingContentHexView.StreamingContentHexView(streamingContentData);
4343
view.markAsRoot();
4444
view.show(document.body);
4545
await raf();
@@ -53,7 +53,7 @@ describeWithEnvironment('RequestBinaryResponseView', () => {
5353
it('shows the updated content of a StreamingContentData', async () => {
5454
const streamingContentData = TextUtils.StreamingContentData.StreamingContentData.from(
5555
new TextUtils.ContentData.ContentData(window.btoa('abc'), /* isBase64 */ true, 'application/octet-stream'));
56-
const view = new Network.RequestBinaryResponseView.RequestBinaryResponseView(streamingContentData);
56+
const view = new SourceFrame.StreamingContentHexView.StreamingContentHexView(streamingContentData);
5757
view.markAsRoot();
5858
view.show(document.body);
5959
await raf();
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// Copyright 2024 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import * as TextUtils from '../../../../models/text_utils/text_utils.js';
6+
import * as LinearMemoryInspectorComponents from '../../../../panels/linear_memory_inspector/components/components.js';
7+
import * as UI from '../../legacy.js';
8+
9+
const MEMORY_TRANSFER_MIN_CHUNK_SIZE = 1000;
10+
11+
/**
12+
* This is a slightly reduced version of `panels/LinearMemoryInspectorPane.LinearMemoryInspectorView.
13+
*
14+
* It's not hooked up to the LinearMemoryInspectorController and it operates on a fixed memory array thats
15+
* known upfront.
16+
*/
17+
class LinearMemoryInspectorView extends UI.Widget.VBox {
18+
#memory = new Uint8Array([0]);
19+
#address = 0;
20+
#inspector = new LinearMemoryInspectorComponents.LinearMemoryInspector.LinearMemoryInspector();
21+
22+
constructor() {
23+
super(false);
24+
this.#inspector.addEventListener(
25+
LinearMemoryInspectorComponents.LinearMemoryInspector.MemoryRequestEvent.eventName,
26+
this.#memoryRequested.bind(this));
27+
this.#inspector.addEventListener(
28+
LinearMemoryInspectorComponents.LinearMemoryInspector.AddressChangedEvent.eventName, event => {
29+
this.#address = event.data;
30+
});
31+
this.contentElement.appendChild(this.#inspector);
32+
}
33+
34+
override wasShown(): void {
35+
this.refreshData();
36+
}
37+
38+
setMemory(memory: Uint8Array): void {
39+
this.#memory = memory;
40+
this.refreshData();
41+
}
42+
43+
refreshData(): void {
44+
// TODO(szuend): The following lines are copied from `LinearMemoryInspectorController`. We can't reuse them
45+
// as depending on a module in `panels/` from a component is a layering violation.
46+
47+
// Provide a chunk of memory that covers the address to show and some before and after
48+
// as 1. the address shown is not necessarily at the beginning of a page and
49+
// 2. to allow for fewer memory requests.
50+
const memoryChunkStart = Math.max(0, this.#address - MEMORY_TRANSFER_MIN_CHUNK_SIZE / 2);
51+
const memoryChunkEnd = memoryChunkStart + MEMORY_TRANSFER_MIN_CHUNK_SIZE;
52+
const memory = this.#memory.slice(memoryChunkStart, memoryChunkEnd);
53+
this.#inspector.data = {
54+
memory,
55+
address: this.#address,
56+
memoryOffset: memoryChunkStart,
57+
outerMemoryLength: this.#memory.length,
58+
hideValueInspector: true,
59+
};
60+
}
61+
62+
#memoryRequested(event: LinearMemoryInspectorComponents.LinearMemoryInspector.MemoryRequestEvent): void {
63+
// TODO(szuend): The following lines are copied from `LinearMemoryInspectorController`. We can't reuse them
64+
// as depending on a module in `panels/` from a component is a layering violation.
65+
66+
const {start, end, address} = event.data;
67+
if (address < start || address >= end) {
68+
throw new Error('Requested address is out of bounds.');
69+
}
70+
71+
// Check that the requested start is within bounds.
72+
// If the requested end is larger than the actual
73+
// memory, it will be automatically capped when
74+
// requesting the range.
75+
if (start < 0 || start > end || start >= this.#memory.length) {
76+
throw new Error('Requested range is out of bounds.');
77+
}
78+
79+
const chunkEnd = Math.max(end, start + MEMORY_TRANSFER_MIN_CHUNK_SIZE);
80+
const memory = this.#memory.slice(start, chunkEnd);
81+
82+
this.#inspector.data = {
83+
memory,
84+
address,
85+
memoryOffset: start,
86+
outerMemoryLength: this.#memory.length,
87+
hideValueInspector: true,
88+
};
89+
}
90+
}
91+
92+
/**
93+
* Adapter for the linear memory inspector that can show a {@link StreamingContentData}.
94+
*/
95+
export class StreamingContentHexView extends LinearMemoryInspectorView {
96+
readonly #streamingContentData: TextUtils.StreamingContentData.StreamingContentData;
97+
98+
constructor(streamingContentData: TextUtils.StreamingContentData.StreamingContentData) {
99+
super();
100+
this.#streamingContentData = streamingContentData;
101+
}
102+
103+
override wasShown(): void {
104+
this.#updateMemoryFromContentData();
105+
this.#streamingContentData.addEventListener(
106+
TextUtils.StreamingContentData.Events.CHUNK_ADDED, this.#updateMemoryFromContentData, this);
107+
108+
// No need to call super.wasShown() as we call super.refreshData() ourselves.
109+
}
110+
111+
override willHide(): void {
112+
super.willHide();
113+
this.#streamingContentData.removeEventListener(
114+
TextUtils.StreamingContentData.Events.CHUNK_ADDED, this.#updateMemoryFromContentData, this);
115+
}
116+
117+
#updateMemoryFromContentData(): void {
118+
const binaryString = window.atob(this.#streamingContentData.content().base64);
119+
const memory = Uint8Array.from(binaryString, m => m.codePointAt(0) as number);
120+
this.setMemory(memory);
121+
}
122+
}

0 commit comments

Comments
 (0)