Skip to content

Commit 7411149

Browse files
masnobleDevtools-frontend LUCI CQ
authored andcommitted
Reland "Have InspectorView remove the Infobar instead of CookieControlsView"
This is a reland of commit 1ac4ec6 Original change's description: > 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]> Bug: 400962121 Change-Id: Id4b4c5a1e46e84877642978c3763af784ff751b6 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6367775 Reviewed-by: Danil Somsikov <[email protected]> Reviewed-by: Kim-Anh Tran <[email protected]> Commit-Queue: Joshua Thomas <[email protected]>
1 parent 9d3c991 commit 7411149

File tree

3 files changed

+69
-24
lines changed

3 files changed

+69
-24
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: 61 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,36 +4,75 @@
44
import {assert} from 'chai';
55

66
import {expectError} from '../../conductor/events.js';
7-
import {click, getBrowserAndPages, waitForAria} from '../../shared/helper.js';
8-
import {reloadDevTools} from '../helpers/cross-tool-helper.js';
7+
import {$textContent, click, getBrowserAndPages, reloadDevTools, waitForAria} from '../../shared/helper.js';
98
import {getDataGridRows} from '../helpers/datagrid-helpers.js';
109
import {
1110
navigateToSecurityTab,
1211
} from '../helpers/security-helpers.js';
1312

14-
describe('The Privacy and security panel', function() {
15-
let preloadScriptId: string;
13+
let preloadScriptId: string;
1614

17-
afterEach(async () => {
18-
// The tests end but DevTools might be still doing things resulting
19-
// in an error caused by the test runner closing or navigating the
20-
// target page.
21-
expectError('Inspected target navigated or closed');
22-
if (!preloadScriptId) {
23-
return;
24-
}
25-
const {frontend} = getBrowserAndPages();
26-
await frontend.removeScriptToEvaluateOnNewDocument(preloadScriptId);
27-
});
15+
async function addPrivacyUIToHostConfig() {
16+
const {frontend} = getBrowserAndPages();
17+
const {identifier} = await frontend.evaluateOnNewDocument(`globalThis.hostConfigForTesting = {
18+
...globalThis.hostConfigForTesting,
19+
devToolsPrivacyUI: {enabled: ${true}},
20+
};`);
21+
preloadScriptId = identifier;
22+
}
23+
24+
async function removeScript() {
25+
// The tests end but DevTools might be still doing things resulting
26+
// in an error caused by the test runner closing or navigating the
27+
// target page.
28+
expectError('Inspected target navigated or closed');
29+
if (!preloadScriptId) {
30+
return;
31+
}
32+
const {frontend} = getBrowserAndPages();
33+
await frontend.removeScriptToEvaluateOnNewDocument(preloadScriptId);
34+
}
2835

36+
describe('The controls tool without the Privacy and security panel open', function() {
2937
beforeEach(async () => {
3038
const {frontend} = getBrowserAndPages();
31-
const {identifier} = await frontend.evaluateOnNewDocument(`globalThis.hostConfigForTesting = {
32-
...globalThis.hostConfigForTesting,
33-
devToolsPrivacyUI: {enabled: ${true}},
34-
};`);
35-
preloadScriptId = identifier;
3639

40+
await addPrivacyUIToHostConfig();
41+
await frontend.evaluate(`(async () => {
42+
const Common = await import('./core/common/common.js');
43+
const setting = Common.Settings.Settings.instance().createSetting('cookie-control-override-enabled', true);
44+
setting.set(true);
45+
})()`);
46+
});
47+
48+
afterEach(async () => {
49+
await removeScript();
50+
});
51+
52+
it('will remove reload bar without privacy module loaded', async () => {
53+
// Reload to give toolbar chance to spawn
54+
await reloadDevTools();
55+
56+
// Infobar should be presenet since the setting was set in the before
57+
const infoBar = await waitForAria('To apply your updated controls, reload the page');
58+
assert.isNotNull(infoBar);
59+
60+
const {target} = getBrowserAndPages();
61+
await target.reload();
62+
63+
// Infobar should be gone after reloading the page
64+
assert.isNull(await $textContent('To apply your updated controls, reload the page'));
65+
});
66+
});
67+
68+
describe('The Privacy and security panel', function() {
69+
before(async () => {
70+
await addPrivacyUIToHostConfig();
71+
await reloadDevTools();
72+
});
73+
74+
after(async () => {
75+
await removeScript();
3776
await reloadDevTools();
3877
});
3978

@@ -43,14 +82,14 @@ describe('The Privacy and security panel', function() {
4382

4483
// Infobar should appear after changing control
4584
const infoBar = await waitForAria('To apply your updated controls, reload the page');
46-
infoBar.evaluate(el => assert.isNotNull(el));
85+
assert.isNotNull(infoBar);
4786

4887
// Allow time for infobar to animate in before clicking the button
4988
await new Promise<void>(resolve => setTimeout(resolve, 550));
5089
await click('dt-close-button', {root: infoBar});
5190

5291
// Infobar should be gone after clicking the close button
53-
infoBar.evaluate(el => assert.isNotNull(el));
92+
assert.isNull(await $textContent('To apply your updated controls, reload the page'));
5493
});
5594

5695
it('filters rows when the search filter is populated', async () => {

0 commit comments

Comments
 (0)