Skip to content

Commit 1ac4ec6

Browse files
masnobleDevtools-frontend LUCI CQ
authored andcommitted
Have InspectorView remove the Infobar instead of CookieControlsView
If CookieControlsView was responsible for listening for a PrimaryPageChanged event and then removing the Infobar, the infobar would not be removed if CookieControlsView was never created to listen in the first place. This is a problem because the Infobar might be created by the InspectorMain entrypoint which does not guarantee that a CookieControlsView will be created as well. By moving the listener to the InspectorView, we guarantee that the Infobar is removed when the user reloads the page without needing to open the Privacy and security panel. Bug: 400962121 Change-Id: I612c4899e218e094fd31a3eda154ec69c20e8035 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6346989 Reviewed-by: Kim-Anh Tran <[email protected]> Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Joshua Thomas <[email protected]>
1 parent 7809edb commit 1ac4ec6

File tree

3 files changed

+104
-61
lines changed

3 files changed

+104
-61
lines changed

front_end/panels/security/CookieControlsView.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,6 @@ export class CookieControlsView extends UI.Widget.VBox {
352352
this.checkGracePeriodActive().catch(error => {
353353
console.error(error);
354354
});
355-
356-
UI.InspectorView.InspectorView.instance().removeDebuggedTabReloadRequiredWarning();
357355
}
358356

359357
async checkGracePeriodActive(event?: Common.EventTarget.EventTargetEvent<SDK.Resource.Resource>): Promise<void> {

front_end/ui/legacy/InspectorView.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import * as Common from '../../core/common/common.js';
3232
import * as Host from '../../core/host/host.js';
3333
import * as i18n from '../../core/i18n/i18n.js';
3434
import * as Root from '../../core/root/root.js';
35+
import * as SDK from '../../core/sdk/sdk.js';
3536
import * as Buttons from '../../ui/components/buttons/buttons.js';
3637
import * as IconButton from '../components/icon_button/icon_button.js';
3738
import * as VisualLogging from '../visual_logging/visual_logging.js';
@@ -501,12 +502,19 @@ export class InspectorView extends VBox implements ViewLocationResolver {
501502
infobar.setCloseCallback(() => {
502503
delete this.reloadRequiredInfobar;
503504
});
505+
506+
SDK.TargetManager.TargetManager.instance().addModelListener(
507+
SDK.ResourceTreeModel.ResourceTreeModel, SDK.ResourceTreeModel.Events.PrimaryPageChanged,
508+
this.removeDebuggedTabReloadRequiredWarning, this);
504509
}
505510
}
506511

507512
removeDebuggedTabReloadRequiredWarning(): void {
508513
if (this.reloadRequiredInfobar) {
509514
this.reloadRequiredInfobar.dispose();
515+
SDK.TargetManager.TargetManager.instance().removeModelListener(
516+
SDK.ResourceTreeModel.ResourceTreeModel, SDK.ResourceTreeModel.Events.PrimaryPageChanged,
517+
this.removeDebuggedTabReloadRequiredWarning, this);
510518
}
511519
}
512520

test/e2e/security/privacy_test.ts

Lines changed: 96 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44
import {assert} from 'chai';
55

66
import {expectError} from '../../conductor/events.js';
7-
import {click, getBrowserAndPages, waitForAria} from '../../shared/helper.js';
7+
import {$textContent, click, getBrowserAndPages, waitForAria} from '../../shared/helper.js';
88
import {reloadDevTools} from '../helpers/cross-tool-helper.js';
99
import {getDataGridRows} from '../helpers/datagrid-helpers.js';
1010
import {
1111
navigateToSecurityTab,
1212
} from '../helpers/security-helpers.js';
1313

14-
describe('The Privacy and security panel', function() {
14+
describe('Privacy tools', function() {
1515
let preloadScriptId: string;
1616

1717
afterEach(async () => {
@@ -37,74 +37,111 @@ describe('The Privacy and security panel', function() {
3737
await reloadDevTools();
3838
});
3939

40-
it('shows reload bar when controls are changed', async () => {
41-
await navigateToSecurityTab(/* privcayEnabled=*/ true);
42-
await click('[aria-label="Temporarily limit third-party cookies, only when DevTools is open"]');
40+
describe('The controls tool without the Privacy and security panel open', function() {
41+
before(async () => {
42+
const {frontend} = getBrowserAndPages();
43+
await frontend.evaluate(`(async () => {
44+
const Common = await import('./core/common/common.js');
45+
const setting = Common.Settings.Settings.instance().createSetting('cookie-control-override-enabled', true);
46+
setting.set(true);
47+
})()`);
4348

44-
// Infobar should appear after changing control
45-
const infoBar = await waitForAria('To apply your updated controls, reload the page');
46-
infoBar.evaluate(el => assert.isNotNull(el));
49+
await reloadDevTools();
50+
});
51+
52+
after(async () => {
53+
const {frontend} = getBrowserAndPages();
54+
await frontend.evaluate(`(async () => {
55+
const Common = await import('./core/common/common.js');
56+
const setting = Common.Settings.Settings.instance().clearAll()
57+
})()`);
58+
59+
await reloadDevTools();
60+
});
4761

48-
// Allow time for infobar to animate in before clicking the button
49-
await new Promise<void>(resolve => setTimeout(resolve, 550));
50-
await click('dt-close-button', {root: infoBar});
62+
it('will remove reload bar without privacy module loaded', async () => {
63+
// Infobar should be presenet since the setting was set in the before
64+
const infoBar = await waitForAria('To apply your updated controls, reload the page');
65+
assert.isNotNull(infoBar);
5166

52-
// Infobar should be gone after clicking the close button
53-
infoBar.evaluate(el => assert.isNotNull(el));
67+
const {target} = getBrowserAndPages();
68+
await target.reload();
69+
70+
// Infobar should be gone after reloading the page
71+
assert.isNull(await $textContent('To apply your updated controls, reload the page'));
72+
});
5473
});
5574

56-
it('filters rows when the search filter is populated', async () => {
57-
await navigateToSecurityTab(/* privcayEnabled=*/ true);
58-
await click('[aria-label="Third-party cookies"]');
75+
describe('The Privacy and security panel', function() {
76+
it('shows reload bar when controls are changed', async () => {
77+
await navigateToSecurityTab(/* privcayEnabled=*/ true);
78+
await click('[aria-label="Temporarily limit third-party cookies, only when DevTools is open"]');
5979

60-
// Populate with test issues to be filtered
61-
const {frontend} = getBrowserAndPages();
62-
frontend.evaluate(() => {
63-
const issue1 = {
64-
code: 'CookieIssue',
65-
details: {
66-
cookieIssueDetails: {
67-
cookie: {
68-
name: 'a',
69-
path: '/',
70-
domain: 'a.test',
80+
// Infobar should appear after changing control
81+
const infoBar = await waitForAria('To apply your updated controls, reload the page');
82+
assert.isNotNull(infoBar);
83+
84+
// Allow time for infobar to animate in before clicking the button
85+
await new Promise<void>(resolve => setTimeout(resolve, 550));
86+
await click('dt-close-button', {root: infoBar});
87+
88+
// Infobar should be gone after clicking the close button
89+
assert.isNull(await $textContent('To apply your updated controls, reload the page'));
90+
});
91+
92+
it('filters rows when the search filter is populated', async () => {
93+
await navigateToSecurityTab(/* privcayEnabled=*/ true);
94+
await click('[aria-label="Third-party cookies"]');
95+
96+
// Populate with test issues to be filtered
97+
const {frontend} = getBrowserAndPages();
98+
frontend.evaluate(() => {
99+
const issue1 = {
100+
code: 'CookieIssue',
101+
details: {
102+
cookieIssueDetails: {
103+
cookie: {
104+
name: 'a',
105+
path: '/',
106+
domain: 'a.test',
107+
},
108+
cookieExclusionReasons: ['ExcludeThirdPartyPhaseout'],
109+
cookieWarningReasons: [],
110+
operation: 'ReadCookie',
111+
cookieUrl: 'a.test',
71112
},
72-
cookieExclusionReasons: ['ExcludeThirdPartyPhaseout'],
73-
cookieWarningReasons: [],
74-
operation: 'ReadCookie',
75-
cookieUrl: 'a.test',
76113
},
77-
},
78-
};
79-
// @ts-expect-error
80-
window.addIssueForTest(issue1);
81-
82-
const issue2 = {
83-
code: 'CookieIssue',
84-
details: {
85-
cookieIssueDetails: {
86-
cookie: {
87-
name: 'b',
88-
path: '/',
89-
domain: 'b.test',
114+
};
115+
// @ts-expect-error
116+
window.addIssueForTest(issue1);
117+
118+
const issue2 = {
119+
code: 'CookieIssue',
120+
details: {
121+
cookieIssueDetails: {
122+
cookie: {
123+
name: 'b',
124+
path: '/',
125+
domain: 'b.test',
126+
},
127+
cookieExclusionReasons: ['ExcludeThirdPartyPhaseout'],
128+
cookieWarningReasons: [],
129+
operation: 'ReadCookie',
130+
cookieUrl: 'b.test',
90131
},
91-
cookieExclusionReasons: ['ExcludeThirdPartyPhaseout'],
92-
cookieWarningReasons: [],
93-
operation: 'ReadCookie',
94-
cookieUrl: 'b.test',
95132
},
96-
},
97-
};
98-
// @ts-expect-error
99-
window.addIssueForTest(issue2);
100-
});
101-
assert.lengthOf(await getDataGridRows(2, undefined, true), 2);
133+
};
134+
// @ts-expect-error
135+
window.addIssueForTest(issue2);
136+
});
137+
assert.lengthOf(await getDataGridRows(2, undefined, true), 2);
102138

103-
const searchFilter = await waitForAria('Filter');
104-
searchFilter.evaluate(el => assert.isNotNull(el));
105-
searchFilter.type('a.test');
139+
const searchFilter = await waitForAria('Filter');
140+
searchFilter.evaluate(el => assert.isNotNull(el));
141+
searchFilter.type('a.test');
106142

107-
// The second issue should be filtered out.
108-
assert.lengthOf(await getDataGridRows(1, undefined, true), 1);
143+
// The second issue should be filtered out.
144+
assert.lengthOf(await getDataGridRows(1, undefined, true), 1);
145+
});
109146
});
110147
});

0 commit comments

Comments
 (0)