Skip to content

Commit

Permalink
Reland "Privacy Sandbox Settings: Pipe cr-dialog close to close funct…
Browse files Browse the repository at this point in the history
…ion"

This is a reland of commit 31f14a5

Original issue was with the test, production code OK. In some build
configs not all events related to close appeared to be queued sync.
CL adjusts logic to wait for final close report, and then element
logic update.

Original change's description:
> Privacy Sandbox Settings: Pipe cr-dialog close to close function
>
> Previously only button closes in the modal dialogs correctly updated
> the view state on the Privacy Sandbox settings page.
>
> This CL adjusts logic so that closes handled by the dialog element,
> such as via ESC, also correctly update the logic.
>
> Bug: 1324926
> Change-Id: Ic155464779814f5fd3b19448acda1c97da096e5f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3644568
> Reviewed-by: Olesia Marukhno <olesiamarukhno@google.com>
> Commit-Queue: Theodore Olsauskas-Warren <sauski@google.com>
> Cr-Commit-Position: refs/heads/main@{#1002654}

(cherry picked from commit 86eb5c7)

Bug: 1324926
Change-Id: Idda949e93aea430779950a14fcb9e520dd618329
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3645074
Commit-Queue: Theodore Olsauskas-Warren <sauski@google.com>
Reviewed-by: Olesia Marukhno <olesiamarukhno@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1003090}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3650612
Auto-Submit: Theodore Olsauskas-Warren <sauski@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#43}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
sauski-alternative authored and Chromium LUCI CQ committed May 17, 2022
1 parent 6debd68 commit 7d11767
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 2 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/resources/settings/privacy_sandbox/app.html
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ <h1 id="page-heading">$i18n{privacySandboxTitle}</h1>
<template is="dom-if"
if="[[!showFragment_(privacySandboxSettingsViewEnum_.MAIN,
privacySandboxSettingsView_)]]" restamp>
<cr-dialog id="dialogWrapper" show-on-attach>
<cr-dialog id="dialogWrapper" show-on-attach on-close="onDialogClose_">
<template id="learnMoreDialog" is="dom-if"
if="[[showFragment_(
privacySandboxSettingsViewEnum_.LEARN_MORE_DIALOG,
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/resources/settings/privacy_sandbox/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ export class PrivacySandboxAppElement extends PrivacySandboxAppElementBase {
}

private onDialogClose_() {
// This function will only be called once, regardless of how the dialog is
// shut (either via ESC or via the button), as in the latter the dialog is
// not "closed", but rather removed from the DOM.
const lastView = this.privacySandboxSettingsView_;
this.privacySandboxSettingsView_ = PrivacySandboxSettingsView.MAIN;
afterNextRender(this, async () => {
Expand Down
35 changes: 34 additions & 1 deletion chrome/test/data/webui/settings/privacy_sandbox_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {CrButtonElement, CrSettingsPrefs, HatsBrowserProxyImpl, loadTimeData, Me

import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {TestBrowserProxy} from 'chrome://webui-test/test_browser_proxy.js';
import {flushTasks, isChildVisible, isVisible} from 'chrome://webui-test/test_util.js';
import {eventToPromise, flushTasks, isChildVisible, isVisible} from 'chrome://webui-test/test_util.js';

import {TestHatsBrowserProxy} from './test_hats_browser_proxy.js';
import {TestMetricsBrowserProxy} from './test_metrics_browser_proxy.js';
Expand Down Expand Up @@ -714,4 +714,37 @@ suite('PrivacySandboxSettings3', function() {
await flushTasks();
assertMainViewVisible();
});

test('directDialogClose', async function() {
// Confirm that closing the dialog directly (as done through the escape key)
// correctly navigates back.
assertMainViewVisible();

// Open a sub page.
page.shadowRoot!.querySelector<HTMLElement>(
'#adPersonalizationRow')!.click();
await flushTasks();
assertAdPersonalizationDialogVisible();

// Close the subpage by closing the modal dialog directly.
const dialogWrapper =
page.shadowRoot!.querySelector<CrDialogElement>('#dialogWrapper');
assertTrue(!!dialogWrapper);
dialogWrapper.close();
// The close() call on the the <cr-dialog> is routed to its internal
// <dialog> element, which then fires close, which the <cr-dialog> then
// fires it's own close event from. Not all of these tasks are necessarily
// queued synchronously. Waiting until the <cr-dialog> fires close, and
// then an additional flushTasks() so the page can react, is required.
await eventToPromise('close', dialogWrapper);
await flushTasks();

assertMainViewVisible();

// Closing the dialog should have reset the view state, such that another
// dialog can be opened.
page.shadowRoot!.querySelector<HTMLElement>('#adMeasurementRow')!.click();
await flushTasks();
assertAdMeasurementDialogVisible();
});
});

0 comments on commit 7d11767

Please sign in to comment.