Skip to content

Commit

Permalink
CloseWebContentsAt returns void
Browse files Browse the repository at this point in the history
Upstream changes to `CloseWebContentsAt` have changed it to a void
return function. This means that in case of test code, it is necessary
for checks to compare the number of tabs before, and after the
`CloseWebContentsAt` calls.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/85b180cc6fb9d81a6578d75254f5e3ccb7ea3f31

commit 85b180cc6fb9d81a6578d75254f5e3ccb7ea3f31
Author: Simon Hangl <[email protected]>
Date:   Thu Jun 29 18:54:44 2023 +0000

    Reland "Prevent non-closable tabs from being closed"

    This is a reland of commit 889fff9f35c578e4da34c2a2ae4ef4c7bb49130a

    CQ_INCLUDE_TRYBOTS=luci.chrome.try:chromeos-betty-pi-arc-chrome
    CQ_INCLUDE_TRYBOTS=luci.chrome.try:chromeos-eve-chrome
    CQ_INCLUDE_TRYBOTS=luci.chrome.try:chromeos-kevin-chrome
    CQ_INCLUDE_TRYBOTS=luci.chrome.try:chromeos-octopus-chrome
    CQ_INCLUDE_TRYBOTS=luci.chrome.try:chromeos-reven-chrome

    Original change's description:
    > Prevent non-closable tabs from being closed
    >
    > This CL implements admin configurable non-closable apps.
    > These apps should neither be closable through closing the only web app
    > tab (which this CL implements) nor by closing the window containing the
    > app (future CL). The check is placed in `TabStripModel::CloseTabs` to
    > ensure the tab cannot be closed, as all paths to close a tab go through
    > this method. The close prevention is achieved by filtering all tabs
    > that cannot be closed from the list of tabs to close.
    >
    > DD: go/preventclose
    > Bug: b/278246367
  • Loading branch information
cdesouza-chromium committed Aug 17, 2023
1 parent d10afc2 commit 76aceb9
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 16 deletions.
12 changes: 8 additions & 4 deletions browser/ephemeral_storage/blob_url_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,10 @@ class BlobUrlBrowserTestBase : public EphemeralStorageBrowserTest {

// Close the first a.com tab, ensure all blobs created there become obsolete
// and can't be fetched.
ASSERT_TRUE(browser()->tab_strip_model()->CloseWebContentsAt(
1, TabCloseTypes::CLOSE_NONE));
const int previous_tab_count = browser()->tab_strip_model()->count();
browser()->tab_strip_model()->CloseWebContentsAt(1,
TabCloseTypes::CLOSE_NONE);
EXPECT_EQ(previous_tab_count - 1, browser()->tab_strip_model()->count());
content::RunAllTasksUntilIdle();
for (size_t idx = 0; idx < a_com2_registered_blobs.size(); ++idx) {
auto* rfh = a_com2_registered_blobs[idx].rfh.get();
Expand Down Expand Up @@ -267,8 +269,10 @@ class BlobUrlBrowserTestBase : public EphemeralStorageBrowserTest {

// Close the first a.com tab, ensure all blobs created there become obsolete
// and can't be fetched.
ASSERT_TRUE(browser()->tab_strip_model()->CloseWebContentsAt(
1, TabCloseTypes::CLOSE_NONE));
const int previous_tab_count = browser()->tab_strip_model()->count();
browser()->tab_strip_model()->CloseWebContentsAt(1,
TabCloseTypes::CLOSE_NONE);
EXPECT_EQ(previous_tab_count - 1, browser()->tab_strip_model()->count());
content::RunAllTasksUntilIdle();
for (size_t idx = 0; idx < a_com2_registered_blobs.size(); ++idx) {
auto* rfh = a_com2_registered_blobs[idx].rfh.get();
Expand Down
23 changes: 14 additions & 9 deletions browser/ephemeral_storage/ephemeral_storage_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,11 @@ WebContents* EphemeralStorageBrowserTest::LoadURLInNewTab(GURL url) {
void EphemeralStorageBrowserTest::CloseWebContents(WebContents* web_contents) {
int tab_index =
browser()->tab_strip_model()->GetIndexOfWebContents(web_contents);
bool was_closed = browser()->tab_strip_model()->CloseWebContentsAt(
tab_index, TabCloseTypes::CLOSE_NONE);
EXPECT_TRUE(was_closed);

const int previous_tab_count = browser()->tab_strip_model()->count();
browser()->tab_strip_model()->CloseWebContentsAt(tab_index,
TabCloseTypes::CLOSE_NONE);
EXPECT_EQ(previous_tab_count - 1, browser()->tab_strip_model()->count());
}

void EphemeralStorageBrowserTest::SetStorageValueInFrame(
Expand Down Expand Up @@ -661,9 +663,11 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest,
// an eTLD.
int tab_index =
browser()->tab_strip_model()->GetIndexOfWebContents(site_a_tab);
bool was_closed = browser()->tab_strip_model()->CloseWebContentsAt(
tab_index, TabCloseTypes::CLOSE_NONE);
EXPECT_TRUE(was_closed);

const int previous_tab_count = browser()->tab_strip_model()->count();
browser()->tab_strip_model()->CloseWebContentsAt(tab_index,
TabCloseTypes::CLOSE_NONE);
EXPECT_EQ(previous_tab_count - 1, browser()->tab_strip_model()->count());
EXPECT_TRUE(WaitForCleanupAfterKeepAlive());

// Navigate the main tab to the same site.
Expand Down Expand Up @@ -1142,9 +1146,10 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageKeepAliveDisabledBrowserTest,
// an eTLD.
int tab_index =
browser()->tab_strip_model()->GetIndexOfWebContents(site_a_tab);
bool was_closed = browser()->tab_strip_model()->CloseWebContentsAt(
tab_index, TabCloseTypes::CLOSE_NONE);
EXPECT_TRUE(was_closed);
const int previous_tab_count = browser()->tab_strip_model()->count();
browser()->tab_strip_model()->CloseWebContentsAt(tab_index,
TabCloseTypes::CLOSE_NONE);
EXPECT_EQ(previous_tab_count - 1, browser()->tab_strip_model()->count());

// Navigate the main tab to the same site.
ASSERT_TRUE(
Expand Down
8 changes: 5 additions & 3 deletions browser/ephemeral_storage/ephemeral_storage_qa_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,11 @@ class EphemeralStorageTest : public InProcessBrowserTest {
"document.getElementById('continue-test-url-step-6').value")
.ExtractString();

ASSERT_TRUE(
tabs_->CloseWebContentsAt(tabs_->GetIndexOfWebContents(original_tab_),
TabCloseTypes::CLOSE_NONE));
const int previous_tab_count = browser()->tab_strip_model()->count();
tabs_->CloseWebContentsAt(tabs_->GetIndexOfWebContents(original_tab_),
TabCloseTypes::CLOSE_NONE);
ASSERT_EQ(previous_tab_count - 1, browser()->tab_strip_model()->count());

EphemeralStorageServiceFactory::GetInstance()
->GetForContext(browser()->profile())
->FireCleanupTimersForTesting();
Expand Down

0 comments on commit 76aceb9

Please sign in to comment.