CVE-2026-93373
Overview
Background
- `sessions::LiveTab` / `sessions::ContentLiveTab`
- an abstraction over a restorable open tab;
ContentLiveTabis thecontent-layer implementation backed by aWebContents, which owns the tab’s lifetime. - `TabRestoreService` / `TabRestoreServiceHelper::RestoreEntryById`
- the service (and its shared helper) that re-opens recently closed tabs, groups, and windows, returning the
LiveTabinstances it created. - `chrome.sessions.restore()` / `SessionsRestoreFunction`
- the extensions API (and its C++ backing function) that lets an extension re-open a closed tab and then inspects the returned tab to build its reply.
- `base::WeakPtr<LiveTab>`
- a pointer that automatically becomes null when the pointee is destroyed, letting callers detect a tab that was freed out from under them.
Root Cause Analysis
TabRestoreServiceHelper::RestoreEntryById collected raw LiveTab* pointers into a std::vector<LiveTab*> live_tabs as it restored tabs, and only afterward called context->ShowBrowserWindow() to show and activate the target browser window. Showing/activating a window can pump a nested event loop, and that reentrancy can synchronously close the just-restored tab (or its window), destroying the backing ContentLiveTab/WebContents while its raw pointer still sits in live_tabs. The stale vector was returned up to SessionsRestoreFunction, whose RestoreMostRecentEntry path only guarded the result with DCHECK(restored_tabs.size()) — a no-op in release builds — before casting restored_tabs[0] to ContentLiveTab* and dereferencing it, yielding a use-after-free of the freed tab.
The fix tracks restored tabs as std::vector<base::WeakPtr<LiveTab>> throughout the pipeline (via the new LiveTab::GetWeakPtr() / ContentLiveTab::GetWeakPtr()), drops any tab that is no longer valid when the restore completes, and changes RestoreEntryById to return std::optional so an invalid id (std::nullopt) is distinguishable from “restored but all tabs closed” (empty vector). SessionsRestoreFunction now replaces the DCHECK with a real restored_tabs.empty() check that returns kNoActiveTabError, so the freed/absent tab is never dereferenced.
LiveTab* pointers across a call (ShowBrowserWindow) that can reentrantly free those very tabs, then dereferenced them; the fix holds the tabs by WeakPtr, filters out any invalidated by that reentrancy, and turns the previously ignored empty result into an explicit error instead of a dereference.Attack Path
- Invoke restore from an extension
An extension with the
sessionspermission callschrome.sessions.restore(), drivingSessionsRestoreFunctionintoTabRestoreService::RestoreMostRecentEntry/RestoreEntryById. - Capture raw pointer before showing the window
TabRestoreServiceHelper::RestoreEntryByIdrestores the tab and pushes its rawLiveTab*intolive_tabs, then callscontext->ShowBrowserWindow(). - Reentrantly close the restored tab
Showing/activating the window pumps nested processing that closes the freshly restored tab (the regression test
RestoreErrorWhenTabClosedDuringRestoredoes exactly this via aTabRestoreServiceObserver), destroying theContentLiveTab/WebContentswhile its raw pointer remains in the returned vector. - Dereference the freed tab
Back in
SessionsRestoreFunction, the release-modeDCHECKis skipped andstatic_cast<ContentLiveTab*>(restored_tabs[0])is dereferenced, reading through the dangling pointer — a use-after-free in the browser process. - Exploitation
Reclaiming the freed
ContentLiveTab/WebContentsallocation with attacker-influenced data to gain a read/write primitive is (inferred); the patch demonstrates only the dangling-pointer dereference.
Impact Assessment
chrome.sessions.restore() can trigger a use-after-free in the privileged browser process by causing a restored tab to close during the restore operation; the precondition is the ability to close the restored tab synchronously during window activation (nested event processing). Any escalation to controlled read/write, memory corruption exploitation, or code execution is (inferred) and not shown by the patch. The vendor advisory rates this high severity and does not report in-the-wild exploitation; NVD scores it CVSS 9.6.Changed Functions
| Function | Change | Notes |
|---|---|---|
ifchrome/browser/extensions/api/sessions/sessions_api.cc |
modified | |
ifcomponents/sessions/core/tab_restore_service_helper.cc |
modified | |
switchcomponents/sessions/core/tab_restore_service_helper.cc |
modified |
Files Changed
components/sessions/core/tab_restore_types.cccomponents/sessions/ios/ios_restore_live_tab.mmcomponents/sessions/content/content_live_tab.hchrome/browser/android/recently_closed_tabs_bridge.cccomponents/sessions/core/tab_restore_service_helper.cccomponents/sessions/core/mock_tab_restore_service.hchrome/browser/sessions/tab_restore_service_unittest.cccomponents/sessions/core/tab_restore_service_impl.hios/chrome/browser/sessions/model/fake_tab_restore_service.mmcomponents/sessions/core/live_tab.hcomponents/sessions/ios/ios_webstate_live_tab.hchrome/browser/extensions/api/sessions/sessions_apitest.cccomponents/sessions/core/tab_restore_service.hcomponents/sessions/content/content_live_tab.cccomponents/sessions/ios/ios_webstate_live_tab.mmchrome/browser/sessions/session_restore_browsertest.ccchrome/browser/sessions/tab_restore_browsertest.cccomponents/sessions/core/tab_restore_service_impl.ccchrome/browser/extensions/api/sessions/sessions_api.cccomponents/sessions/core/tab_restore_service_helper.hcomponents/sessions/core/tab_restore_types.hios/chrome/browser/sessions/model/fake_tab_restore_service.hcomponents/sessions/ios/ios_restore_live_tab.h
Audit Directions
- Raw pointers cached across reentrancy-capable callsAudit any code that stores raw
LiveTab*,WebContents*,Browser*, or tab pointers and then calls a UI/activation routine (ShowBrowserWindow, window activation, focus changes) that can pump a nested event loop before the pointer is used; preferWeakPtror handle-based access across such calls. - `DCHECK`-only guards before a dereferenceSearch for
DCHECK(...size()),DCHECK(ptr), or similar assertions immediately preceding an indexed access or dereference; these vanish in release builds and must be replaced with real runtime checks on any path reachable from untrusted callers such as extension APIs. - Empty-vs-invalid conflation in return valuesReview APIs that fold “no such entry” and “operation completed but produced nothing” into one empty-collection return; callers frequently assume at least one element, so distinguishing the cases (as with the new
std::optional) is needed wherever restore/lookup results are indexed without a length check.
Patch
From 54db51dbd55d1cab1a3dc053222f4f20a0f38477 Mon Sep 17 00:00:00 2001
From: Joe Mason <joenotcharles@google.com>
Date: Mon, 14 Sep 2026 22:09:24 -0700
Subject: [PATCH] Reland "Return only live tabs from TabRestoreService restore operations"
This reverts commit a467c37711d44556022a49dbbbce6b1d4bda8964.
Reason for revert: Fixed MSAN errors
Original change's description:
> Revert "Return only live tabs from TabRestoreService restore operations"
>
> This reverts commit b10218380b91df1c63d4eb1a8d4ad50adfa22ea7.
>
> Reason for revert: Newly added tests are failing
>
> Failure Link:
> https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/61349/overview
>
> https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20MSan%20Tests/64914/overview
>
>
> Original change's description:
> > Return only live tabs from TabRestoreService restore operations
> >
> > TabRestoreService restore methods previously collected raw LiveTab
> > pointers before showing and activating the target browser window.
> > If showing the window triggers nested event processing that closes
> > restored tabs or the window before the operation completes, the
> > returned collection could include pointers to destroyed tabs.
> >
> > Track restored tabs using WeakPtr throughout the restore pipeline and
> > filter out any tabs that are no longer valid upon completion. In
> > addition, update SessionsRestoreFunction to handle empty restore results
> > gracefully by returning an error instead of assuming a tab is always
> > present.
> >
> > Fixed: 553132214
> > Change-Id: I007a5f97eecdd692080f23d866ee6cdf80c2a248
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8383396
> > Reviewed-by: Darryl James <dljames@chromium.org>
> > Commit-Queue: Joe Mason <joenotcharles@google.com>
> > Auto-Submit: Joe Mason <joenotcharles@google.com>
> > Owners-Override: Gabriel Charette <gab@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1696548}
>
> Bug: 553132214
> Change-Id: I9fc29f5113c263f1f3cb0c3c3ef5c951ffd6de63
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8405065
> Commit-Queue: Lingqi Chi <lingqi@chromium.org>
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Auto-Submit: Lingqi Chi <lingqi@chromium.org>
> Owners-Override: Lingqi Chi <lingqi@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1697070}
Bug: 553132214
Fixed: 553132214
Change-Id: If281660e94ad3fbf5174416e83581adabebc7aea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8405169
Reviewed-by: Darryl James <dljames@chromium.org>
Owners-Override: Gabriel Charette <gab@chromium.org>
Commit-Queue: Joe Mason <joenotcharles@google.com>
Cr-Commit-Position: refs/heads/main@{#1697819}
---
diff --git a/chrome/browser/android/recently_closed_tabs_bridge.cc b/chrome/browser/android/recently_closed_tabs_bridge.cc
index 6878063b..87f18c5d 100644
--- a/chrome/browser/android/recently_closed_tabs_bridge.cc
+++ b/chrome/browser/android/recently_closed_tabs_bridge.cc
@@ -4,6 +4,9 @@
#include "chrome/browser/android/recently_closed_tabs_bridge.h"
+#include <optional>
+#include <vector>
+
#include "base/android/jni_array.h"
#include "base/android/jni_string.h"
#include "base/android/token_android.h"
@@ -333,11 +336,11 @@
}
AndroidLiveTabContextRestoreWrapper restore_context(model);
- std::vector<sessions::LiveTab*> restored_tabs =
+ std::optional<std::vector<sessions::LiveTab*>> restored_tabs =
tab_restore_service_->RestoreEntryById(
&restore_context, entry_id,
static_cast<WindowOpenDisposition>(j_disposition));
- return !restored_tabs.empty();
+ return !restored_tabs.value_or({}).empty();
}
bool RecentlyClosedTabsBridge::OpenRecentlyClosedEntry(
@@ -355,12 +358,12 @@
}
AndroidLiveTabContextRestoreWrapper restore_context(model);
- std::vector<sessions::LiveTab*> restored_tabs =
+ std::optional<std::vector<sessions::LiveTab*>> restored_tabs =
tab_restore_service_->RestoreEntryById(
&restore_context, SessionID::FromSerializedValue(entry_session_id),
WindowOpenDisposition::NEW_BACKGROUND_TAB);
RestoreAndroidTabGroups(env, model, restore_context.GetTabGroups());
- return !restored_tabs.empty();
+ return !restored_tabs.value_or({}).empty();
}
bool RecentlyClosedTabsBridge::OpenMostRecentlyClosedEntry(JNIEnv* env,
@@ -381,12 +384,12 @@
// AndroidLiveTabContext. `restore_context` is required to rebuild groups
// information. To avoid this just use the first entry in entries when
// restoring.
- std::vector<sessions::LiveTab*> restored_tabs =
+ std::optional<std::vector<sessions::LiveTab*>> restored_tabs =
tab_restore_service_->RestoreEntryById(
&restore_context, tab_restore_service_->entries().front()->id,
WindowOpenDisposition::NEW_BACKGROUND_TAB);
RestoreAndroidTabGroups(env, model, restore_context.GetTabGroups());
- return !restored_tabs.empty();
+ return !restored_tabs.value_or({}).empty();
}
void RecentlyClosedTabsBridge::ClearRecentlyClosedEntries(JNIEnv* env) {
diff --git a/chrome/browser/extensions/api/sessions/sessions_api.cc b/chrome/browser/extensions/api/sessions/sessions_api.cc
index ef44b5a..c25f1353 100644
--- a/chrome/browser/extensions/api/sessions/sessions_api.cc
+++ b/chrome/browser/extensions/api/sessions/sessions_api.cc
@@ -749,7 +749,9 @@
}
std::vector<sessions::LiveTab*> restored_tabs =
tab_restore_service->RestoreMostRecentEntry(context);
- DCHECK(restored_tabs.size());
+ if (restored_tabs.empty()) {
+ return RespondNow(Error(kNoActiveTabError));
+ }
sessions::ContentLiveTab* first_tab =
static_cast<sessions::ContentLiveTab*>(restored_tabs[0]);
@@ -926,17 +928,19 @@
if (!context) {
return RespondNow(Error(kNoLiveTabContextError));
}
- std::vector<sessions::LiveTab*> restored_tabs =
+ std::optional<std::vector<sessions::LiveTab*>> restored_tabs =
tab_restore_service->RestoreEntryById(
context, SessionID::FromSerializedValue(session_id.id()),
WindowOpenDisposition::UNKNOWN);
- // If the ID is invalid, restored_tabs will be empty.
- if (restored_tabs.empty()) {
+ if (!restored_tabs.has_value()) {
return RespondNow(Error(kInvalidSessionIdError, session_id.ToString()));
}
+ if (restored_tabs->empty()) {
+ return RespondNow(Error(kNoActiveTabError));
+ }
sessions::ContentLiveTab* first_tab =
- static_cast<sessions::ContentLiveTab*>(restored_tabs[0]);
+ static_cast<sessions::ContentLiveTab*>((*restored_tabs)[0]);
// Retrieve the window through any of the tabs in restored_tabs.
if (is_window) {
diff --git a/components/sessions/content/content_live_tab.cc b/components/sessions/content/content_live_tab.cc
index ab86ddab..2d4396f 100644
--- a/components/sessions/content/content_live_tab.cc
+++ b/components/sessions/content/content_live_tab.cc
@@ -69,6 +69,10 @@
return serialized_ua_override;
}
+base::WeakPtr<LiveTab> ContentLiveTab::GetWeakPtr() {
+ return weak_ptr_factory_.GetWeakPtr();
+}
+
WEB_CONTENTS_USER_DATA_KEY_IMPL(ContentLiveTab);
} // namespace sessions
diff --git a/components/sessions/content/content_live_tab.h b/components/sessions/content/content_live_tab.h
index a5ec2743..b6b3f91 100644
--- a/components/sessions/content/content_live_tab.h
+++ b/components/sessions/content/content_live_tab.h
@@ -6,6 +6,7 @@
#define COMPONENTS_SESSIONS_CONTENT_CONTENT_LIVE_TAB_H_
#include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
#include "base/supports_user_data.h"
#include "components/sessions/content/content_serialized_navigation_builder.h"
#include "components/sessions/core/live_tab.h"
@@ -43,6 +44,7 @@
std::unique_ptr<tab_restore::PlatformSpecificTabData>
GetPlatformSpecificTabData() override;
SerializedUserAgentOverride GetUserAgentOverride() override;
+ base::WeakPtr<LiveTab> GetWeakPtr() override;
private:
friend class content::WebContentsUserData<ContentLiveTab>;
@@ -55,6 +57,8 @@
content::NavigationController& navigation_controller() {
return GetWebContents().GetController();
}
+
+ base::WeakPtrFactory<ContentLiveTab> weak_ptr_factory_{this};
};
} // namespace sessions
diff --git a/components/sessions/core/live_tab.h b/components/sessions/core/live_tab.h
index d236139..eee207f 100644
--- a/components/sessions/core/live_tab.h
+++ b/components/sessions/core/live_tab.h
@@ -5,6 +5,7 @@
#ifndef COMPONENTS_SESSIONS_CORE_LIVE_TAB_H_
#define COMPONENTS_SESSIONS_CORE_LIVE_TAB_H_
+#include "base/memory/weak_ptr.h"
#include "components/sessions/core/serialized_navigation_entry.h"
#include "components/sessions/core/serialized_user_agent_override.h"
#include "components/sessions/core/session_id.h"
@@ -40,6 +41,9 @@
// Returns the user agent override, if any.
virtual SerializedUserAgentOverride GetUserAgentOverride() = 0;
+
+ // Returns a WeakPtr that is invalidated when the tab is destroyed.
+ virtual base::WeakPtr<LiveTab> GetWeakPtr() = 0;
};
} // namespace sessions
diff --git a/components/sessions/core/mock_tab_restore_service.h b/components/sessions/core/mock_tab_restore_service.h
index 24d6645..aa57903b 100644
--- a/components/sessions/core/mock_tab_restore_service.h
+++ b/components/sessions/core/mock_tab_restore_service.h
@@ -5,6 +5,7 @@
#ifndef COMPONENTS_SESSIONS_CORE_MOCK_TAB_RESTORE_SERVICE_H_
#define COMPONENTS_SESSIONS_CORE_MOCK_TAB_RESTORE_SERVICE_H_
+#include <optional>
#include <vector>
#include "components/sessions/core/tab_restore_service.h"
@@ -109,7 +110,7 @@
(int num_to_remove),
(override));
- MOCK_METHOD(std::vector<sessions::LiveTab*>,
+ MOCK_METHOD(std::optional<std::vector<sessions::LiveTab*>>,
RestoreEntryById,
(sessions::LiveTabContext * context,
SessionID id,
diff --git a/components/sessions/core/tab_restore_service.h b/components/sessions/core/tab_restore_service.h
index 24ef1341..8e5e08d8 100644
--- a/components/sessions/core/tab_restore_service.h
+++ b/components/sessions/core/tab_restore_service.h
@@ -113,7 +113,8 @@
// Restores the most recently closed entry. Does nothing if there are no
// entries to restore. If the most recently restored entry is a tab, it is
- // added to |context|. Returns the LiveTab instances of the restored tab(s).
+ // added to |context|. Returns the LiveTab instances of the restored tab(s)
+ // that are still open when the restore completes.
virtual std::vector<LiveTab*> RestoreMostRecentEntry(
LiveTabContext* context) = 0;
@@ -124,12 +125,15 @@
virtual void RemoveLeastRecentlyUsedEntries(int num_to_remove) = 0;
// Restores an entry by id. If there is no entry with an id matching |id|,
- // this does nothing. If |context| is NULL, this creates a new window for the
- // entry. |disposition| is respected, but the attributes (tabstrip index,
- // browser window) of the tab when it was closed will be respected if
- // disposition is UNKNOWN. Returns the LiveTab instances of the restored
- // tab(s).
- virtual std::vector<LiveTab*> RestoreEntryById(
+ // this does nothing and returns nullopt. Otherwise, returns the LiveTab
+ // instances of the restored tab(s) that are still open when the restore
+ // completes.
+ //
+ // If |context| is NULL, this creates a new window for the entry.
+ // |disposition| is respected, but the attributes (tabstrip index, browser
+ // window) of the tab when it was closed will be respected if disposition is
+ // UNKNOWN.
+ virtual std::optional<std::vector<LiveTab*>> RestoreEntryById(
LiveTabContext* context,
SessionID id,
WindowOpenDisposition disposition) = 0;
diff --git a/components/sessions/core/tab_restore_service_helper.cc b/components/sessions/core/tab_restore_service_helper.cc
index 841b971..f291342 100644
--- a/components/sessions/core/tab_restore_service_helper.cc
+++ b/components/sessions/core/tab_restore_service_helper.cc
@@ -661,7 +661,8 @@
return std::vector<LiveTab*>();
}
return RestoreEntryById(context, entries_.front()->id,
- WindowOpenDisposition::UNKNOWN);
+ WindowOpenDisposition::UNKNOWN)
+ .value();
}
void TabRestoreServiceHelper::RemoveEntryById(SessionID id) {
@@ -692,7 +693,7 @@
SessionID id,
LiveTabContext* context,
WindowOpenDisposition disposition,
- std::vector<LiveTab*>* live_tabs) {
+ std::vector<base::WeakPtr<LiveTab>>* live_tabs) {
// 1. Determine if `id` corresponds to a tab. If so, restore the tab.
bool found_tab_to_delete = false;
SessionID::id_type restored_tab_browser_id;
@@ -709,7 +710,9 @@
context = RestoreTab(tab, context, disposition,
sessions::tab_restore::WINDOW, &restored_tab,
/*is_restoring_group_or_window=*/false);
- live_tabs->push_back(restored_tab);
+ if (restored_tab) {
+ live_tabs->push_back(restored_tab->GetWeakPtr());
+ }
// Cleanup.
std::optional<tab_groups::TabGroupId> group_id = tab.group;
@@ -781,7 +784,9 @@
DCHECK_EQ(new_context, context);
}
context = new_context;
- live_tabs->push_back(restored_tab);
+ if (restored_tab) {
+ live_tabs->push_back(restored_tab->GetWeakPtr());
+ }
window.tabs.erase(window.tabs.begin() + tab_i);
}
@@ -833,7 +838,7 @@
RestoreTab(tab, context, disposition, sessions::tab_restore::WINDOW,
&restored_tab, /*is_restoring_group_or_window=*/false);
if (restored_tab) {
- live_tabs->push_back(restored_tab);
+ live_tabs->push_back(restored_tab->GetWeakPtr());
restored_tab_map[tab.id] = restored_tab;
}
@@ -898,14 +903,13 @@
return group_mapping;
}
-std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
+std::optional<std::vector<LiveTab*>> TabRestoreServiceHelper::RestoreEntryById(
LiveTabContext* context,
SessionID id,
WindowOpenDisposition disposition) {
auto entry_iterator = GetEntryIteratorById(id);
if (entry_iterator == entries_.end()) {
- // Don't hoark here, we allow an invalid id.
- return std::vector<LiveTab*>();
+ return std::nullopt;
}
if (observer_) {
@@ -923,7 +927,12 @@
// |context| will be NULL in cases where one isn't already available (eg,
// when invoked on Mac OS X with no windows open). In this case, create a
// new browser into which we restore the tabs.
- std::vector<LiveTab*> live_tabs;
+ //
+ // The restored tabs are tracked with WeakPtrs because restoring involves
+ // deletion-capable calls (e.g. showing and activating browser windows) that
+ // can synchronously close restored tabs, or even the window they were
+ // restored into, before this method returns.
+ std::vector<base::WeakPtr<LiveTab>> live_tabs;
switch (entry.type) {
case tab_restore::Type::TAB: {
auto& tab = static_cast<const Tab&>(entry);
@@ -937,7 +946,9 @@
LiveTab* restored_tab = nullptr;
context = RestoreTab(tab, context, disposition, entry.type, &restored_tab,
/*is_restoring_group_or_window=*/false);
- live_tabs.push_back(restored_tab);
+ if (restored_tab) {
+ live_tabs.push_back(restored_tab->GetWeakPtr());
+ }
context->ShowBrowserWindow();
break;
}
@@ -1000,7 +1011,7 @@
client_->OnTabRestored(
tab->navigations.at(tab->current_navigation_index)
.virtual_url());
- live_tabs.push_back(restored_tab);
+ live_tabs.push_back(restored_tab->GetWeakPtr());
restored_tab_map[tab->id] = restored_tab;
}
@@ -1096,7 +1107,7 @@
*tab.get(), context->GetTabCount(), group.tabs[0]->id == tab->id,
/*restored_from_group_or_window_context=*/true, entry.type);
if (restored_tab) {
- live_tabs.push_back(restored_tab);
+ live_tabs.push_back(restored_tab->GetWeakPtr());
restored_tab_map[tab->id] = restored_tab;
}
if (tab->split_id.has_value()) {
@@ -1135,7 +1146,9 @@
... (file diff truncated)
Regression Test / PoC
diff --git a/chrome/browser/extensions/api/sessions/sessions_apitest.cc b/chrome/browser/extensions/api/sessions/sessions_apitest.cc
index 21687b64..9f83766 100644
--- a/chrome/browser/extensions/api/sessions/sessions_apitest.cc
+++ b/chrome/browser/extensions/api/sessions/sessions_apitest.cc
@@ -6,6 +6,7 @@
#include <array>
#include <memory>
+#include <string>
#include <utility>
#include "base/command_line.h"
@@ -14,7 +15,9 @@
#include "base/functional/callback.h"
#include "base/functional/callback_helpers.h"
#include "base/memory/ptr_util.h"
+#include "base/memory/raw_ptr.h"
#include "base/path_service.h"
+#include "base/scoped_observation.h"
#include "base/strings/pattern.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h"
@@ -47,6 +50,8 @@
#include "chrome/test/base/testing_browser_process.h"
#include "components/sessions/content/content_live_tab.h"
#include "components/sessions/core/session_id.h"
+#include "components/sessions/core/tab_restore_service.h"
+#include "components/sessions/core/tab_restore_service_observer.h"
#include "components/sync/base/client_tag_hash.h"
#include "components/sync/engine/data_type_activation_response.h"
#include "components/sync/model/data_type_activation_request.h"
@@ -58,6 +63,7 @@
#include "components/sync/test/mock_data_type_worker.h"
#include "components/sync_sessions/session_store.h"
#include "components/sync_sessions/session_sync_service.h"
+#include "components/tabs/public/tab_interface.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "extensions/browser/api_test_utils.h"
@@ -65,8 +71,10 @@
#include "extensions/buildflags/buildflags.h"
#include "extensions/common/extension_builder.h"
#include "google_apis/gaia/gaia_id.h"
+#include "third_party/abseil-cpp/absl/container/flat_hash_set.h"
#include "ui/base/base_window.h"
#include "ui/base/page_transition_types.h"
+#include "url/gurl.h"
#if BUILDFLAG(IS_CHROMEOS)
#include "ash/constants/ash_switches.h"
@@ -595,6 +603,92 @@
<< error;
}
+namespace {
+
+// Closes all tabs created by an in-progress restore as soon as the tab restore
+// service reports a change, i.e. before the restore operation returns.
+class ClosingTabRestoreServiceObserver
+ : public sessions::TabRestoreServiceObserver {
+ public:
+ ClosingTabRestoreServiceObserver(sessions::TabRestoreService* service,
+ BrowserWindowInterface* browser)
+ : browser_(browser) {
+ observation_.Observe(service);
+ for (tabs::TabInterface* tab :
+ TabListInterface::From(browser_)->GetAllTabs()) {
+ initial_tab_handles_.insert(tab->GetHandle());
+ }
+ }
+
+ ~ClosingTabRestoreServiceObserver() override = default;
+
+ size_t num_closed_tabs() const { return num_closed_tabs_; }
+
+ // sessions::TabRestoreServiceObserver:
+ void TabRestoreServiceChanged(sessions::TabRestoreService* service) override {
+ if (!service->IsRestoring() || num_closed_tabs_ > 0) {
+ return;
+ }
+ TabListInterface* tab_list = TabListInterface::From(browser_);
+ for (tabs::TabInterface* tab : tab_list->GetAllTabs()) {
+ if (!initial_tab_handles_.contains(tab->GetHandle())) {
+ ++num_closed_tabs_;
+ tab_list->CloseTab(tab->GetHandle());
+ }
+ }
+ }
+
+ void TabRestoreServiceDestroyed(
+ sessions::TabRestoreService* service) override {
+ observation_.Reset();
+ }
+
+ private:
+ raw_ptr<BrowserWindowInterface> browser_;
+ base::ScopedObservation<sessions::TabRestoreService,
+ sessions::TabRestoreServiceObserver>
+ observation_{this};
+ absl::flat_hash_set<tabs::TabInterface::Handle> initial_tab_handles_;
+ size_t num_closed_tabs_ = 0;
+};
+
+} // namespace
+
+// Tests that chrome.sessions.restore() returns an error rather than stale tab
+// information when the restored tab is closed again before the restore
+// operation finishes.
+IN_PROC_BROWSER_TEST_F(ExtensionSessionsTest,
+ RestoreErrorWhenTabClosedDuringRestore) {
+ BrowserWindowInterface* browser = browser_window_interface();
+ TabListInterface* tab_list = TabListInterface::From(browser);
+ ASSERT_TRUE(tab_list);
+
+ // Open a tab, navigate it, and close it to create a recently closed tab
+ // entry. The navigation is needed for the closed tab to be persisted in the
+ // tab restore service.
+ tabs::TabInterface* tab = tab_list->OpenTab(GURL("about:blank"), /*index=*/1);
+ ASSERT_TRUE(tab);
+ ASSERT_TRUE(NavigateToURL(tab->GetContents(), GURL("chrome://version/")));
+ tab_list->CloseTab(tab->GetHandle());
+
+ sessions::TabRestoreService* tab_restore_service =
+ TabRestoreServiceFactory::GetForProfile(GetProfile());
+ ASSERT_TRUE(tab_restore_service);
+ ASSERT_EQ(1u, tab_restore_service->entries().size());
+ const int tab_count = tab_list->GetTabCount();
+
+ // Close the restored tab while the restore is still in progress.
+ ClosingTabRestoreServiceObserver closing_observer(tab_restore_service,
+ browser);
+ std::string error = utils::RunFunctionAndReturnError(
+ CreateFunction<SessionsRestoreFunction>(true).get(), /*args=*/"[]",
+ GetProfile());
+
+ EXPECT_EQ(closing_observer.num_closed_tabs(), 1u);
+ EXPECT_EQ("No active tab.", error);
+ EXPECT_EQ(tab_count, tab_list->GetTabCount());
+}
+
// Tests chrome.sessions.getRecentlyClosed() for windows. Opens a second browser
// window with two tabs, closes it, then calls the extension API function and
// verifies one window with two tabs was recently closed.
diff --git a/chrome/browser/sessions/session_restore_browsertest.cc b/chrome/browser/sessions/session_restore_browsertest.cc
index e3620151..60c3c3c 100644
--- a/chrome/browser/sessions/session_restore_browsertest.cc
+++ b/chrome/browser/sessions/session_restore_browsertest.cc
@@ -8,6 +8,7 @@
#include <array>
#include <memory>
+#include <optional>
#include <ranges>
#include <set>
#include <string_view>
@@ -783,11 +784,13 @@
if (tab.navigations[0].virtual_url() == url2) {
timestamp = tab.navigations[0].timestamp();
http_status_code = tab.navigations[0].http_status_code();
- std::vector<sessions::LiveTab*> content = service->RestoreEntryById(
- nullptr, tab.id, WindowOpenDisposition::UNKNOWN);
- ASSERT_EQ(1U, content.size());
+ std::optional<std::vector<sessions::LiveTab*>> content =
+ service->RestoreEntryById(nullptr, tab.id,
+ WindowOpenDisposition::UNKNOWN);
+ ASSERT_TRUE(content.has_value());
+ ASSERT_EQ(1U, content->size());
sessions::ContentLiveTab* live_tab =
- static_cast<sessions::ContentLiveTab*>(content[0]);
+ static_cast<sessions::ContentLiveTab*>((*content)[0]);
ASSERT_TRUE(live_tab);
EXPECT_EQ(url2, live_tab->GetWebContents().GetURL());
break;
diff --git a/chrome/browser/sessions/tab_restore_browsertest.cc b/chrome/browser/sessions/tab_restore_browsertest.cc
index cf14bf9e..d7f5e03 100644
--- a/chrome/browser/sessions/tab_restore_browsertest.cc
+++ b/chrome/browser/sessions/tab_restore_browsertest.cc
@@ -2368,7 +2368,7 @@
// Restore the window.
browser_created_observer.emplace();
- std::vector<sessions::LiveTab*> restored_window_tabs =
+ std::optional<std::vector<sessions::LiveTab*>> restored_window_tabs =
service->RestoreEntryById(BrowserLiveTabContext::From(second_browser),
entries.front()->id,
WindowOpenDisposition::NEW_FOREGROUND_TAB);
@@ -2379,7 +2379,8 @@
// We will opt to open the saved group instead of individually restoring all
// of the tabs in the group one at a time. Because of this, RestoreEntryById
// will only return one tab as being restored.
- ASSERT_EQ(1u, restored_window_tabs.size());
+ ASSERT_TRUE(restored_window_tabs.has_value());
+ ASSERT_EQ(1u, restored_window_tabs->size());
ASSERT_NE(second_browser, third_browser);
ASSERT_EQ(3, third_browser->GetTabStripModel()->count());
diff --git a/chrome/browser/sessions/tab_restore_service_unittest.cc b/chrome/browser/sessions/tab_restore_service_unittest.cc
index e4678a67..a4d0995 100644
--- a/chrome/browser/sessions/tab_restore_service_unittest.cc
+++ b/chrome/browser/sessions/tab_restore_service_unittest.cc
@@ -9,11 +9,13 @@
#include <optional>
#include <string>
#include <utility>
+#include <vector>
#include "base/compiler_specific.h"
#include "base/functional/bind.h"
#include "base/memory/ptr_util.h"
#include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
@@ -39,12 +41,14 @@
#include "components/sessions/content/content_live_tab.h"
#include "components/sessions/content/content_test_helper.h"
#include "components/sessions/content/session_tab_helper.h"
+#include "components/sessions/core/live_tab.h"
#include "components/sessions/core/serialized_navigation_entry_test_helper.h"
#include "components/sessions/core/session_id.h"
#include "components/sessions/core/session_types.h"
#include "components/sessions/core/tab_restore_service_client.h"
#include "components/sessions/core/tab_restore_service_impl.h"
#include "components/sessions/core/tab_restore_service_observer.h"
+#include "components/sessions/core/tab_restore_types.h"
#include "components/split_tabs/split_tab_id.h"
#include "components/split_tabs/split_tab_visual_data.h"
#include "components/tab_groups/tab_group_id.h"
@@ -74,7 +78,10 @@
using sessions::SerializedNavigationEntryTestHelper;
using ::testing::_;
+using ::testing::IsEmpty;
+using ::testing::Optional;
using ::testing::Return;
+using ::testing::UnorderedElementsAre;
class MockLiveTab : public sessions::LiveTab {
public:
@@ -92,6 +99,13 @@
GetPlatformSpecificTabData,
std::unique_ptr<sessions::tab_restore::PlatformSpecificTabData>());
MOCK_METHOD0(GetUserAgentOverride, sessions::SerializedUserAgentOverride());
+
+ base::WeakPtr<sessions::LiveTab> GetWeakPtr() override {
+ return weak_ptr_factory_.GetWeakPtr();
+ }
+
+ private:
+ base::WeakPtrFactory<MockLiveTab> weak_ptr_factory_{this};
};
class MockLiveTabContext : public sessions::LiveTabContext {
@@ -1729,3 +1743,102 @@
EXPECT_EQ(2, restored_tab_count);
EXPECT_EQ(max_entries - 1, service_->entries().size());
}
+
+// Restoring a window with a tab that's closed again before the restore
+// completes (e.g. while the browser window is being shown) excludes that tab
+// from the returned vector.
+TEST_F(TabRestoreServiceImplWithMockClientTest,
+ RestoreWindowEntryOmitsTabsClosedDuringRestore) {
+ // Create a window entry with three tabs.
+ auto window = std::make_unique<Window>();
+ for (int i = 0; i < 3; ++i) {
+ auto tab = std::make_unique<Tab>();
+ tab->navigations.push_back(ContentTestHelper::CreateNavigation(
+ base::StringPrintf("http://%d", i), base::NumberToString(i)));
+ tab->current_navigation_index = 0;
+ window->tabs.push_back(std::move(tab));
+ }
+ window->selected_tab_index = 0;
+ const SessionID entry_id = window->id;
+ mutable_entries()->push_back(std::move(window));
+
+ testing::NiceMock<MockLiveTabContext> restore_context;
+ ON_CALL(restore_context, GetSessionID())
+ .WillByDefault(Return(SessionID::NewUnique()));
+ ON_CALL(*mock_tab_restore_service_client_,
+ CreateLiveTabContext(_, _, _, _, _, _, _, _))
+ .WillByDefault(Return(&restore_context));
+
+ auto restored_tab1 = std::make_unique<testing::NiceMock<MockLiveTab>>();
+ auto restored_tab2 = std::make_unique<testing::NiceMock<MockLiveTab>>();
+ EXPECT_CALL(restore_context, AddRestoredTab(_, _, _, _, _))
+ .WillOnce(Return(restored_tab1.get()))
+ .RetiresOnSaturation();
+ EXPECT_CALL(restore_context, AddRestoredTab(_, _, _, _, _))
+ .WillOnce(Return(restored_tab2.get()))
+ .RetiresOnSaturation();
+ // A tab that can't be created for a tab entry should be excluded from the
+ // list, rather than included as a nullptr.
+ EXPECT_CALL(restore_context, AddRestoredTab(_, _, _, _, _))
+ .WillOnce(Return(nullptr))
+ .RetiresOnSaturation();
+
+ // Close the first restored tab while the window is being shown, which happens
+ // before RestoreEntryById() returns.
... (truncated)