Skip to content

Commit 9138bce

Browse files
danilsomsikovDevtools-frontend LUCI CQ
authored andcommitted
Reduce the scope of frame details view functions.
Mostly replaced Frame with individual fields in parameters and rearranged the control flow accordingly. But also replaced home-grown DOMNode link with a reusable widget and simplified StackTrace component data. Bug: 407750239 Change-Id: I6957d6c3f91e71f67db6372dcf587cdc0f724d92 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7156362 Auto-Submit: Danil Somsikov <[email protected]> Reviewed-by: Wolfgang Beyer <[email protected]> Commit-Queue: Danil Somsikov <[email protected]>
1 parent 6a3d30b commit 9138bce

File tree

7 files changed

+143
-166
lines changed

7 files changed

+143
-166
lines changed

front_end/panels/application/components/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ devtools_ui_module("components") {
5858
"../../../core/root:bundle",
5959
"../../../core/sdk:bundle",
6060
"../../../models/bindings:bundle",
61+
"../../../panels/common:bundle",
6162
"../../../panels/network/forward:bundle",
6263
"../../../third_party/csp_evaluator:bundle",
6364
"../../../ui/components/adorners:bundle",

front_end/panels/application/components/FrameDetailsView.test.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ const makeFrame = (target: SDK.Target.Target) => {
4343
Protocol.Page.GatedAPIFeatures.SharedArrayBuffersTransferAllowed],
4444
getOwnerDOMNodeOrDocument: () => Promise.resolve({
4545
nodeName: () => 'iframe',
46+
nodeType: () => Node.ELEMENT_NODE,
47+
pseudoType: () => undefined,
48+
isViewTransitionPseudoNode: () => false,
49+
nodeNameInCorrectCase: () => 'iframe',
50+
getAttribute: () => null,
4651
}),
4752
resourceTreeModel: () => target.model(SDK.ResourceTreeModel.ResourceTreeModel),
4853
getCreationStackTraceData: () => ({
@@ -200,16 +205,16 @@ describeWithMockConnection('FrameDetailsView', () => {
200205
'Measure Memory',
201206
]);
202207

203-
const values = getCleanTextContentFromElements(component.shadowRoot, 'devtools-report-value');
208+
const values = [...component.shadowRoot.querySelectorAll('devtools-report-value')].map(v => v.deepInnerText());
204209
assert.deepEqual(values, [
205210
'https://www.example.com/path/page.html',
206211
'https://www.example.com',
207-
'<iframe>',
208-
'',
209-
'',
210-
'',
212+
'iframe',
213+
'function1\n\xA0@\xA0www.example.com/script.js:16',
214+
'root',
215+
'ad-script1.js:1',
211216
'/ad-script2.$script',
212-
'Yes\xA0Localhost is always a secure context',
217+
'Yes\nLocalhost is always a secure context',
213218
'Yes',
214219
'none',
215220
'same-origin',
@@ -219,7 +224,7 @@ object-src: 'none'
219224
script-src: 'strict-dynamic', 'unsafe-inline', https:, http:, 'nonce-GsVjHiIoejpPhMPOHDQZ90yc9eJn1s', 'unsafe-eval'
220225
report-uri: https://www.example.com/csp`,
221226
'available, transferable',
222-
'available\xA0Learn more',
227+
'available\nLearn more',
223228
]);
224229

225230
const stackTrace = getElementWithinComponent(

front_end/panels/application/components/FrameDetailsView.ts

Lines changed: 66 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import * as SDK from '../../../core/sdk/sdk.js';
1515
import * as Protocol from '../../../generated/protocol.js';
1616
import * as Bindings from '../../../models/bindings/bindings.js';
1717
import * as Workspace from '../../../models/workspace/workspace.js';
18+
import * as PanelCommon from '../../../panels/common/common.js';
1819
import * as NetworkForward from '../../../panels/network/forward/forward.js';
1920
import * as CspEvaluator from '../../../third_party/csp_evaluator/csp_evaluator.js';
2021
import * as Buttons from '../../../ui/components/buttons/buttons.js';
@@ -87,10 +88,6 @@ const UIStrings = {
8788
* @description Related node label in Timeline UIUtils of the Performance panel
8889
*/
8990
ownerElement: 'Owner Element',
90-
/**
91-
* @description Title for a link to the Elements panel
92-
*/
93-
clickToOpenInElementsPanel: 'Click to open in Elements panel',
9491
/**
9592
* @description Title for ad frame type field
9693
*/
@@ -287,6 +284,7 @@ interface FrameDetailsViewInput {
287284
protocolMonitorExperimentEnabled: boolean;
288285
trials: Protocol.Page.OriginTrial[]|null;
289286
securityIsolationInfo?: Promise<Protocol.Network.SecurityIsolationStatus|null>;
287+
onRevealInNetwork?: () => void;
290288
onRevealInSources: () => void;
291289
}
292290

@@ -350,26 +348,22 @@ function renderDocumentSection(input: FrameDetailsViewInput): LitTemplate {
350348
<devtools-report-key>${i18nString(UIStrings.url)}</devtools-report-key>
351349
<devtools-report-value>
352350
<div class="inline-items">
353-
${maybeRenderSourcesLinkForURL(input.frame, input.onRevealInSources)}
354-
${maybeRenderNetworkLinkForURL(input.frame)}
351+
${!input.frame?.unreachableUrl() ? renderSourcesLinkForURL(input.onRevealInSources) : nothing}
352+
${input.onRevealInNetwork ? renderNetworkLinkForURL(input.onRevealInNetwork) : nothing}
355353
<div class="text-ellipsis" title=${input.frame.url}>${input.frame.url}</div>
356354
</div>
357355
</devtools-report-value>
358-
${maybeRenderUnreachableURL(input.frame)}
359-
${maybeRenderOrigin(input.frame)}
360-
${until(input.linkTargetDOMNode?.then?.(value => renderOwnerElement(input.frame, value)), nothing)}
361-
${maybeRenderCreationStacktrace(input.frame)}
362-
${maybeRenderAdStatus(input.frame)}
363-
${maybeRenderCreatorAdScriptAncestry(input.frame, input.target, input.adScriptAncestry)}
356+
${maybeRenderUnreachableURL(input.frame?.unreachableUrl())}
357+
${maybeRenderOrigin(input.frame?.securityOrigin)}
358+
${until(input.linkTargetDOMNode?.then?.(value => renderOwnerElement(value)), nothing)}
359+
${maybeRenderCreationStacktrace(input.frame.getCreationStackTraceData())}
360+
${maybeRenderAdStatus(input.frame?.adFrameType(), input.frame?.adFrameStatus())}
361+
${maybeRenderCreatorAdScriptAncestry(input.frame?.adFrameType(), input.target, input.adScriptAncestry)}
364362
<devtools-report-divider></devtools-report-divider>
365363
`;
366364
}
367365

368-
function maybeRenderSourcesLinkForURL(
369-
frame: SDK.ResourceTreeModel.ResourceTreeFrame|null, onRevealInSources: () => void): LitTemplate {
370-
if (!frame || frame.unreachableUrl()) {
371-
return nothing;
372-
}
366+
function renderSourcesLinkForURL(onRevealInSources: () => void): LitTemplate {
373367
return renderIconLink(
374368
'label',
375369
i18nString(UIStrings.clickToOpenInSourcesPanel),
@@ -378,92 +372,75 @@ function maybeRenderSourcesLinkForURL(
378372
);
379373
}
380374

381-
function maybeRenderNetworkLinkForURL(frame: SDK.ResourceTreeModel.ResourceTreeFrame|null): LitTemplate {
382-
if (frame) {
383-
const resource = frame.resourceForURL(frame.url);
384-
if (resource?.request) {
385-
const request = resource.request;
386-
return renderIconLink('arrow-up-down-circle', i18nString(UIStrings.clickToOpenInNetworkPanel), () => {
387-
const requestLocation = NetworkForward.UIRequestLocation.UIRequestLocation.tab(
388-
request, NetworkForward.UIRequestLocation.UIRequestTabs.HEADERS_COMPONENT);
389-
return Common.Revealer.reveal(requestLocation);
390-
}, 'reveal-in-network');
391-
}
392-
}
393-
return nothing;
375+
function renderNetworkLinkForURL(onRevealInNetwork: () => void): LitTemplate {
376+
return renderIconLink(
377+
'arrow-up-down-circle', i18nString(UIStrings.clickToOpenInNetworkPanel), onRevealInNetwork, 'reveal-in-network');
394378
}
395379

396-
function maybeRenderUnreachableURL(frame: SDK.ResourceTreeModel.ResourceTreeFrame|null): LitTemplate {
397-
if (!frame?.unreachableUrl()) {
380+
function maybeRenderUnreachableURL(unreachableUrl: Platform.DevToolsPath.UrlString): LitTemplate {
381+
if (!unreachableUrl) {
398382
return nothing;
399383
}
400384
return html`
401385
<devtools-report-key>${i18nString(UIStrings.unreachableUrl)}</devtools-report-key>
402386
<devtools-report-value>
403387
<div class="inline-items">
404-
${renderNetworkLinkForUnreachableURL(frame)}
405-
<div class="text-ellipsis" title=${frame.unreachableUrl()}>${frame.unreachableUrl()}</div>
388+
${renderNetworkLinkForUnreachableURL(unreachableUrl)}
389+
<div class="text-ellipsis" title=${unreachableUrl}>${unreachableUrl}</div>
406390
</div>
407391
</devtools-report-value>
408392
`;
409393
}
410394

411-
function renderNetworkLinkForUnreachableURL(frame: SDK.ResourceTreeModel.ResourceTreeFrame|null): LitTemplate {
412-
if (frame) {
413-
const unreachableUrl = Common.ParsedURL.ParsedURL.fromString(frame.unreachableUrl());
414-
if (unreachableUrl) {
415-
return renderIconLink(
416-
'arrow-up-down-circle',
417-
i18nString(UIStrings.clickToOpenInNetworkPanelMight),
418-
():
419-
void => {
420-
void Common.Revealer.reveal(NetworkForward.UIFilter.UIRequestFilter.filters([
421-
{
422-
filterType: NetworkForward.UIFilter.FilterType.Domain,
423-
filterValue: unreachableUrl.domain(),
424-
},
425-
{
426-
filterType: null,
427-
filterValue: unreachableUrl.path,
428-
},
429-
]));
430-
},
431-
'unreachable-url.reveal-in-network',
432-
);
433-
}
395+
function renderNetworkLinkForUnreachableURL(unreachableUrlString: Platform.DevToolsPath.UrlString): LitTemplate {
396+
const unreachableUrl = Common.ParsedURL.ParsedURL.fromString(unreachableUrlString);
397+
if (unreachableUrl) {
398+
return renderIconLink(
399+
'arrow-up-down-circle',
400+
i18nString(UIStrings.clickToOpenInNetworkPanelMight),
401+
():
402+
void => {
403+
void Common.Revealer.reveal(NetworkForward.UIFilter.UIRequestFilter.filters([
404+
{
405+
filterType: NetworkForward.UIFilter.FilterType.Domain,
406+
filterValue: unreachableUrl.domain(),
407+
},
408+
{
409+
filterType: null,
410+
filterValue: unreachableUrl.path,
411+
},
412+
]));
413+
},
414+
'unreachable-url.reveal-in-network',
415+
);
434416
}
435417
return nothing;
436418
}
437419

438-
function maybeRenderOrigin(frame: SDK.ResourceTreeModel.ResourceTreeFrame|null): LitTemplate {
439-
if (frame?.securityOrigin && frame?.securityOrigin !== '://') {
420+
function maybeRenderOrigin(securityOrigin: string|null): LitTemplate {
421+
if (securityOrigin && securityOrigin !== '://') {
440422
return html`
441423
<devtools-report-key>${i18nString(UIStrings.origin)}</devtools-report-key>
442424
<devtools-report-value>
443-
<div class="text-ellipsis" title=${frame.securityOrigin}>${frame.securityOrigin}</div>
425+
<div class="text-ellipsis" title=${securityOrigin}>${securityOrigin}</div>
444426
</devtools-report-value>
445427
`;
446428
}
447429
return nothing;
448430
}
449431

450-
function renderOwnerElement(
451-
frame: SDK.ResourceTreeModel.ResourceTreeFrame|null, linkTargetDOMNode: SDK.DOMModel.DOMNode|null): LitTemplate {
432+
function renderOwnerElement(linkTargetDOMNode: SDK.DOMModel.DOMNode|null): LitTemplate {
452433
if (linkTargetDOMNode) {
453434
// Disabled until https://crbug.com/1079231 is fixed.
454435
// clang-format off
455436
return html`
456437
<devtools-report-key>${i18nString(UIStrings.ownerElement)}</devtools-report-key>
457438
<devtools-report-value class="without-min-width">
458439
<div class="inline-items">
459-
<button class="link text-link" role="link" tabindex=0 title=${i18nString(UIStrings.clickToOpenInElementsPanel)}
460-
@mouseenter=${() => frame?.highlight()}
461-
@mouseleave=${() => SDK.OverlayModel.OverlayModel.hideDOMNodeHighlight()}
462-
@click=${() => Common.Revealer.reveal(linkTargetDOMNode)}
463-
jslog=${VisualLogging.action('reveal-in-elements').track({click: true})}
464-
>
465-
&lt;${linkTargetDOMNode.nodeName().toLocaleLowerCase()}&gt;
466-
</button>
440+
<devtools-widget .widgetConfig=${widgetConfig(PanelCommon.DOMLinkifier.DOMNodeLink, {
441+
node: linkTargetDOMNode
442+
})}>
443+
</devtools-widget>
467444
</div>
468445
</devtools-report-value>
469446
`;
@@ -472,8 +449,10 @@ function renderOwnerElement(
472449
return nothing;
473450
}
474451

475-
function maybeRenderCreationStacktrace(frame: SDK.ResourceTreeModel.ResourceTreeFrame|null): LitTemplate {
476-
const creationStackTraceData = frame?.getCreationStackTraceData();
452+
function maybeRenderCreationStacktrace(
453+
creationStackTraceData:
454+
{creationStackTrace: Protocol.Runtime.StackTrace|null, creationStackTraceTarget: SDK.Target.Target}|
455+
null): LitTemplate {
477456
if (creationStackTraceData?.creationStackTrace) {
478457
// Disabled until https://crbug.com/1079231 is fixed.
479458
// clang-format off
@@ -484,7 +463,7 @@ function maybeRenderCreationStacktrace(frame: SDK.ResourceTreeModel.ResourceTree
484463
jslog=${VisualLogging.section('frame-creation-stack-trace')}
485464
>
486465
<devtools-resources-stack-trace .data=${{
487-
frame,
466+
creationStackTraceData,
488467
buildStackTraceRows: Components.JSPresentationUtils.buildStackTraceRowsForLegacyRuntimeStackTrace,
489468
} as StackTraceData}>
490469
</devtools-resources-stack-trace>
@@ -516,17 +495,15 @@ function getAdFrameExplanationString(explanation: Protocol.Page.AdFrameExplanati
516495
}
517496
}
518497

519-
function maybeRenderAdStatus(frame: SDK.ResourceTreeModel.ResourceTreeFrame|null): LitTemplate {
520-
if (!frame) {
521-
return nothing;
522-
}
523-
const adFrameType = frame.adFrameType();
524-
if (adFrameType === Protocol.Page.AdFrameType.None) {
498+
function maybeRenderAdStatus(
499+
adFrameType: Protocol.Page.AdFrameType|undefined,
500+
adFrameStatus: Protocol.Page.AdFrameStatus|undefined): LitTemplate {
501+
if (adFrameType === undefined || adFrameType === Protocol.Page.AdFrameType.None) {
525502
return nothing;
526503
}
527504
const typeStrings = getAdFrameTypeStrings(adFrameType);
528505
const rows = [html`<div title=${typeStrings.description}>${typeStrings.value}</div>`];
529-
for (const explanation of frame.adFrameStatus()?.explanations || []) {
506+
for (const explanation of adFrameStatus?.explanations || []) {
530507
rows.push(html`<div>${getAdFrameExplanationString(explanation)}</div>`);
531508
}
532509

@@ -543,12 +520,8 @@ function maybeRenderAdStatus(frame: SDK.ResourceTreeModel.ResourceTreeFrame|null
543520
}
544521

545522
function maybeRenderCreatorAdScriptAncestry(
546-
frame: SDK.ResourceTreeModel.ResourceTreeFrame|null, target: SDK.Target.Target|null,
523+
adFrameType: Protocol.Page.AdFrameType|null, target: SDK.Target.Target|null,
547524
adScriptAncestry: Protocol.Page.AdScriptAncestry|null): LitTemplate {
548-
if (!frame) {
549-
return nothing;
550-
}
551-
const adFrameType = frame.adFrameType();
552525
if (adFrameType === Protocol.Page.AdFrameType.None) {
553526
return nothing;
554527
}
@@ -922,6 +895,7 @@ export class FrameDetailsReportView extends LegacyWrapper.LegacyWrapper.Wrappabl
922895
const networkManager = frame.resourceTreeModel().target().model(SDK.NetworkManager.NetworkManager);
923896
const securityIsolationInfo = networkManager?.getSecurityIsolationStatus(frame.id);
924897
const linkTargetDOMNode = frame.getOwnerDOMNodeOrDocument();
898+
const frameRequest = frame.resourceForURL(frame.url)?.request;
925899
const input = {
926900
frame,
927901
target: this.#target,
@@ -932,6 +906,13 @@ export class FrameDetailsReportView extends LegacyWrapper.LegacyWrapper.Wrappabl
932906
linkTargetDOMNode,
933907
trials: await frame.getOriginTrials(),
934908
securityIsolationInfo,
909+
onRevealInNetwork: frameRequest ?
910+
() => {
911+
const requestLocation = NetworkForward.UIRequestLocation.UIRequestLocation.tab(
912+
frameRequest, NetworkForward.UIRequestLocation.UIRequestTabs.HEADERS_COMPONENT);
913+
return Common.Revealer.reveal(requestLocation);
914+
} :
915+
undefined,
935916
onRevealInSources: async () => {
936917
const sourceCode = this.#uiSourceCodeForFrame(frame);
937918
if (sourceCode) {

0 commit comments

Comments
 (0)