Skip to content

Commit ea366f5

Browse files
yaoxiachromiumDevtools-frontend LUCI CQ
authored andcommitted
[Ad Tagging] Display 'Root Script Filterlist Rule' in DevTools Frontend
What: Display a 'Root Script Filterlist Rule' entry when applicable. Why: This provides more devtools clarity and coverage of why frames are labeled as ads. Screenshot: https://issues.chromium.org/421927612#attachment66387147 Sibling browser backend change: https://crrev.com/c/6598776 Bug: 421927612 Change-Id: I3075549a80ef35785bc6fe53268a9f5da0f305ce Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6614838 Commit-Queue: Yao Xiao <yaoxia@chromium.org> Reviewed-by: Wolfgang Beyer <wolfi@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org>
1 parent 6ab4487 commit ea366f5

File tree

5 files changed

+37
-23
lines changed

5 files changed

+37
-23
lines changed

front_end/core/sdk/ResourceTreeModel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -783,9 +783,9 @@ export class ResourceTreeFrame {
783783
return this.#domainAndRegistryInternal;
784784
}
785785

786-
async getAdScriptAncestryIds(frameId: Protocol.Page.FrameId): Promise<Protocol.Page.AdScriptId[]|null> {
786+
async getAdScriptAncestry(frameId: Protocol.Page.FrameId): Promise<Protocol.Page.AdScriptAncestry|null> {
787787
const res = await this.#model.agent.invoke_getAdScriptAncestry({frameId});
788-
return res.adScriptAncestry?.ancestryChain || null;
788+
return res.adScriptAncestry || null;
789789
}
790790

791791
get securityOrigin(): string|null {

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

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const makeFrame = (target: SDK.Target.Target) => {
3131
unreachableUrl: () => '',
3232
adFrameType: () => Protocol.Page.AdFrameType.None,
3333
adFrameStatus: () => undefined,
34-
getAdScriptAncestryIds: () => null,
34+
getAdScriptAncestry: () => null,
3535
resourceForURL: () => null,
3636
isSecureContext: () => true,
3737
isCrossOriginIsolated: () => true,
@@ -138,16 +138,19 @@ describeWithMockConnection('FrameDetailsView', () => {
138138
const frame = makeFrame(target);
139139
frame.adFrameType = () => Protocol.Page.AdFrameType.Root;
140140
frame.parentFrame = () => ({
141-
getAdScriptAncestryIds: () => ([
142-
{
143-
scriptId: '123' as Protocol.Runtime.ScriptId,
144-
debuggerId: '42' as Protocol.Runtime.UniqueDebuggerId,
145-
},
146-
{
147-
scriptId: '456' as Protocol.Runtime.ScriptId,
148-
debuggerId: '42' as Protocol.Runtime.UniqueDebuggerId,
149-
}
150-
]),
141+
getAdScriptAncestry: () => ({
142+
ancestryChain: [
143+
{
144+
scriptId: '123' as Protocol.Runtime.ScriptId,
145+
debuggerId: '42' as Protocol.Runtime.UniqueDebuggerId,
146+
},
147+
{
148+
scriptId: '456' as Protocol.Runtime.ScriptId,
149+
debuggerId: '42' as Protocol.Runtime.UniqueDebuggerId,
150+
}
151+
],
152+
rootScriptFilterlistRule: '/ad-script2.$script',
153+
}),
151154
} as unknown as SDK.ResourceTreeModel.ResourceTreeFrame);
152155
const networkManager = target.model(SDK.NetworkManager.NetworkManager);
153156
assert.exists(networkManager);
@@ -183,6 +186,7 @@ describeWithMockConnection('FrameDetailsView', () => {
183186
'Frame Creation Stack Trace',
184187
'Ad Status',
185188
'Creator Ad Script Ancestry',
189+
'Root Script Filterlist Rule',
186190
'Secure Context',
187191
'Cross-Origin Isolated',
188192
'Cross-Origin Embedder Policy (COEP)',
@@ -200,6 +204,7 @@ describeWithMockConnection('FrameDetailsView', () => {
200204
'',
201205
'',
202206
'',
207+
'/ad-script2.$script',
203208
'Yes\xA0Localhost is always a secure context',
204209
'Yes',
205210
'None',

front_end/panels/application/components/FrameDetailsView.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,10 @@ const UIStrings = {
252252
*@description Label for the link(s) to the ad script(s) that led to this frame's creation.
253253
*/
254254
creatorAdScriptAncestry: 'Creator Ad Script Ancestry',
255+
/**
256+
*@description Label for the filterlist rule that identified the root script in 'Creator Ad Script Ancestry' as an ad.
257+
*/
258+
rootScriptFilterlistRule: 'Root Script Filterlist Rule',
255259
/**
256260
*@description Text describing the absence of a value.
257261
*/
@@ -269,7 +273,7 @@ const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
269273
export interface FrameDetailsReportViewData {
270274
frame: SDK.ResourceTreeModel.ResourceTreeFrame;
271275
target?: SDK.Target.Target;
272-
adScriptAncestryIds: Protocol.Page.AdScriptId[]|null;
276+
adScriptAncestry: Protocol.Page.AdScriptAncestry|null;
273277
}
274278

275279
export class FrameDetailsReportView extends LegacyWrapper.LegacyWrapper.WrappableComponent {
@@ -281,7 +285,7 @@ export class FrameDetailsReportView extends LegacyWrapper.LegacyWrapper.Wrappabl
281285
#permissionsPolicySectionData: PermissionsPolicySectionData = {policies: [], showDetails: false};
282286
#originTrialTreeView: OriginTrialTreeView = new OriginTrialTreeView();
283287
#linkifier = new Components.Linkifier.Linkifier();
284-
#adScriptAncestryIds: Protocol.Page.AdScriptId[]|null = null;
288+
#adScriptAncestry: Protocol.Page.AdScriptAncestry|null = null;
285289

286290
constructor(frame: SDK.ResourceTreeModel.ResourceTreeFrame) {
287291
super();
@@ -295,15 +299,15 @@ export class FrameDetailsReportView extends LegacyWrapper.LegacyWrapper.Wrappabl
295299
}
296300

297301
override async render(): Promise<void> {
298-
const results = await this.#frame?.parentFrame()?.getAdScriptAncestryIds(this.#frame?.id);
299-
if (Array.isArray(results) && results.length > 0) {
300-
this.#adScriptAncestryIds = results;
302+
const result = await this.#frame?.parentFrame()?.getAdScriptAncestry(this.#frame?.id);
303+
if (result && result.ancestryChain.length > 0) {
304+
this.#adScriptAncestry = result;
301305

302306
// Obtain the Target associated with the first ad script, because in most scenarios all
303307
// scripts share the same debuggerId. However, discrepancies might arise when content scripts
304308
// from browser extensions are involved. We will monitor the debugging experiences and revisit
305309
// this approach if it proves problematic.
306-
const firstScript = this.#adScriptAncestryIds[0];
310+
const firstScript = this.#adScriptAncestry.ancestryChain[0];
307311
const debuggerModel = firstScript?.debuggerId ?
308312
await SDK.DebuggerModel.DebuggerModel.modelForDebuggerId(firstScript.debuggerId) :
309313
null;
@@ -600,11 +604,11 @@ export class FrameDetailsReportView extends LegacyWrapper.LegacyWrapper.Wrappabl
600604
return Lit.nothing;
601605
}
602606

603-
if (!this.#target || !this.#adScriptAncestryIds || this.#adScriptAncestryIds.length === 0) {
607+
if (!this.#target || !this.#adScriptAncestry || this.#adScriptAncestry.ancestryChain.length === 0) {
604608
return Lit.nothing;
605609
}
606610

607-
const rows = this.#adScriptAncestryIds.map(adScriptId => {
611+
const rows = this.#adScriptAncestry.ancestryChain.map(adScriptId => {
608612
const adScriptLinkElement = this.#linkifier.linkifyScriptLocation(
609613
this.#target,
610614
adScriptId.scriptId || null,
@@ -618,6 +622,8 @@ export class FrameDetailsReportView extends LegacyWrapper.LegacyWrapper.Wrappabl
618622
return html`<div>${adScriptLinkElement}</div>`;
619623
});
620624

625+
const shouldRenderFilterlistRule = (this.#adScriptAncestry.rootScriptFilterlistRule !== undefined);
626+
621627
// Disabled until https://crbug.com/1079231 is fixed.
622628
// clang-format off
623629
return html`
@@ -627,6 +633,10 @@ export class FrameDetailsReportView extends LegacyWrapper.LegacyWrapper.Wrappabl
627633
{rows, title: i18nString(UIStrings.creatorAdScriptAncestry)} as ExpandableList.ExpandableList.ExpandableListData}>
628634
</devtools-expandable-list>
629635
</devtools-report-value>
636+
${shouldRenderFilterlistRule ? html`
637+
<devtools-report-key>${i18nString(UIStrings.rootScriptFilterlistRule)}</devtools-report-key>
638+
<devtools-report-value jslog=${VisualLogging.section('root-script-filterlist-rule')}>${this.#adScriptAncestry.rootScriptFilterlistRule}</devtools-report-value>
639+
` : Lit.nothing}
630640
`;
631641
// clang-format on
632642
}

front_end/ui/visual_logging/KnownContextValues.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2978,6 +2978,7 @@ export const knownContextValues = new Set([
29782978
'right',
29792979
'ro',
29802980
'roboto',
2981+
'root-script-filterlist-rule',
29812982
'rotate',
29822983
'row-gap',
29832984
'row-rule',

test/conductor/events.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ const ALLOWED_ASSERTION_FAILURES = [
3939
// neterror.js started serving sourcemaps and we're requesting it unnecessarily.
4040
'Request Network.loadNetworkResource failed. {"code":-32602,"message":"Unsupported URL scheme"}',
4141
'Fetch API cannot load chrome-error://chromewebdata/neterror.rollup.js.map. URL scheme "chrome-error" is not supported.',
42-
// crbug.com/413061397: Temporarily ignore until CfT roll includes https://crrev.com/c/6480657
43-
'Request Page.getAdScriptAncestryIds failed. {"code":-32601,"message":"\'Page.getAdScriptAncestryIds\' wasn\'t found"}',
4442
'Request Storage.getAffectedUrlsForThirdPartyCookieMetadata failed. {"code":-32603,"message":"Internal error"}',
4543
];
4644

0 commit comments

Comments
 (0)