From 3eb94b72ba6659fb27d59a71f1f9ba945219bd15 Mon Sep 17 00:00:00 2001 From: Calvin Date: Mon, 1 Apr 2024 12:09:01 -0400 Subject: [PATCH] feat: Options parameter for `Session.clearData` API (#41355) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Options parameter for `Session.clearData` API * Consolidate & curate data type categories * Update docs for better typing * off-by-one typo * refactor to use `std::shared_ptr` instead of `base::RefCounted` * fix compile errors * std::enable_shared_from_this didn't work 🤷 * Refine docs with defaults --- docs/api/session.md | 35 +- shell/browser/api/electron_api_session.cc | 332 +++++++++++++++--- shell/browser/api/electron_api_session.h | 3 +- spec/api-session-spec.ts | 92 ++++- .../api/localstorage-and-indexeddb.html | 50 +++ 5 files changed, 449 insertions(+), 63 deletions(-) create mode 100644 spec/fixtures/api/localstorage-and-indexeddb.html diff --git a/docs/api/session.md b/docs/api/session.md index ffd3ee33d9eb..7b24a227241c 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -1428,23 +1428,34 @@ is emitted. Returns `string | null` - The absolute file system path where data for this session is persisted on disk. For in memory sessions this returns `null`. -#### `ses.clearData()` +#### `ses.clearData([options])` + +* `options` Object (optional) + * `dataTypes` String[] (optional) - The types of data to clear. By default, this will clear all types of data. + * `backgroundFetch` - Background Fetch + * `cache` - Cache + * `cookies` - Cookies + * `downloads` - Downloads + * `fileSystems` - File Systems + * `indexedDB` - IndexedDB + * `localStorage` - Local Storage + * `serviceWorkers` - Service Workers + * `webSQL` - WebSQL + * `origins` String[] (optional) - Clear data for only these origins. Cannot be used with `excludeOrigins`. + * `excludeOrigins` String[] (optional) - Clear data for all origins except these ones. Cannot be used with `origins`. + * `avoidClosingConnections` boolean (optional) - Skips deleting cookies that would close current network connections. (Default: `false`) + * `originMatchingMode` String (optional) - The behavior for matching data to origins. + * `third-parties-included` (default) - Storage is matched on origin in first-party contexts and top-level-site in third-party contexts. + * `origin-in-all-contexts` - Storage is matched on origin only in all contexts. Returns `Promise` - resolves when all data has been cleared. -This method clears many different types of data, inlcuding: - -* Cache -* Cookies -* Downloads -* IndexedDB -* Local Storage -* Service Workers -* And more... +Clears various different types of data. This method clears more types of data and is more thourough than the -`clearStorageData` method, however it is currently less configurable than that -method. +`clearStorageData` method. + +**Note:** Cookies are stored at a broader scope than origins. When removing cookies and filtering by `origins` (or `excludeOrigins`), the cookies will be removed at the [registrable domain](https://url.spec.whatwg.org/#host-registrable-domain) level. For example, clearing cookies for the origin `https://really.specific.origin.example.com/` will end up clearing all cookies for `example.com`. Clearing cookies for the origin `https://my.website.example.co.uk/` will end up clearing all cookies for `example.co.uk`. For more information, refer to Chromium's [`BrowsingDataRemover` interface](https://source.chromium.org/chromium/chromium/src/+/main:content/public/browser/browsing_data_remover.h). diff --git a/shell/browser/api/electron_api_session.cc b/shell/browser/api/electron_api_session.cc index 2b0865756a97..297118b0a225 100644 --- a/shell/browser/api/electron_api_session.cc +++ b/shell/browser/api/electron_api_session.cc @@ -12,6 +12,7 @@ #include #include "base/command_line.h" +#include "base/containers/fixed_flat_map.h" #include "base/files/file_enumerator.h" #include "base/files/file_path.h" #include "base/files/file_util.h" @@ -33,12 +34,14 @@ #include "content/browser/code_cache/generated_code_cache_context.h" // nogncheck #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/browsing_data_filter_builder.h" #include "content/public/browser/browsing_data_remover.h" #include "content/public/browser/download_item_utils.h" #include "content/public/browser/download_manager_delegate.h" #include "content/public/browser/network_service_instance.h" #include "content/public/browser/storage_partition.h" #include "gin/arguments.h" +#include "gin/converter.h" #include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/self_owned_receiver.h" #include "net/base/completion_repeating_callback.h" @@ -86,6 +89,7 @@ #include "third_party/blink/public/common/storage_key/storage_key.h" #include "third_party/blink/public/mojom/mediastream/media_stream.mojom.h" #include "ui/base/l10n/l10n_util.h" +#include "url/origin.h" #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) #include "extensions/browser/extension_registry.h" @@ -106,6 +110,8 @@ #endif using content::BrowserThread; +using content::BrowsingDataFilterBuilder; +using content::BrowsingDataRemover; using content::StoragePartition; namespace { @@ -117,25 +123,23 @@ struct ClearStorageDataOptions { }; uint32_t GetStorageMask(const std::vector& storage_types) { + static constexpr auto Lookup = + base::MakeFixedFlatMap( + {{"cookies", StoragePartition::REMOVE_DATA_MASK_COOKIES}, + {"filesystem", StoragePartition::REMOVE_DATA_MASK_FILE_SYSTEMS}, + {"indexdb", StoragePartition::REMOVE_DATA_MASK_INDEXEDDB}, + {"localstorage", StoragePartition::REMOVE_DATA_MASK_LOCAL_STORAGE}, + {"shadercache", StoragePartition::REMOVE_DATA_MASK_SHADER_CACHE}, + {"websql", StoragePartition::REMOVE_DATA_MASK_WEBSQL}, + {"serviceworkers", + StoragePartition::REMOVE_DATA_MASK_SERVICE_WORKERS}, + {"cachestorage", StoragePartition::REMOVE_DATA_MASK_CACHE_STORAGE}}); + uint32_t storage_mask = 0; for (const auto& it : storage_types) { auto type = base::ToLowerASCII(it); - if (type == "cookies") - storage_mask |= StoragePartition::REMOVE_DATA_MASK_COOKIES; - else if (type == "filesystem") - storage_mask |= StoragePartition::REMOVE_DATA_MASK_FILE_SYSTEMS; - else if (type == "indexdb") - storage_mask |= StoragePartition::REMOVE_DATA_MASK_INDEXEDDB; - else if (type == "localstorage") - storage_mask |= StoragePartition::REMOVE_DATA_MASK_LOCAL_STORAGE; - else if (type == "shadercache") - storage_mask |= StoragePartition::REMOVE_DATA_MASK_SHADER_CACHE; - else if (type == "websql") - storage_mask |= StoragePartition::REMOVE_DATA_MASK_WEBSQL; - else if (type == "serviceworkers") - storage_mask |= StoragePartition::REMOVE_DATA_MASK_SERVICE_WORKERS; - else if (type == "cachestorage") - storage_mask |= StoragePartition::REMOVE_DATA_MASK_CACHE_STORAGE; + if (Lookup.contains(type)) + storage_mask |= Lookup.at(type); } return storage_mask; } @@ -152,38 +156,202 @@ uint32_t GetQuotaMask(const std::vector& quota_types) { return quota_mask; } -constexpr content::BrowsingDataRemover::DataType kClearDataTypeAll = ~0ULL; -constexpr content::BrowsingDataRemover::OriginType kClearOriginTypeAll = - content::BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB | - content::BrowsingDataRemover::ORIGIN_TYPE_PROTECTED_WEB; +constexpr BrowsingDataRemover::DataType kClearDataTypeAll = ~0ULL; +constexpr BrowsingDataRemover::OriginType kClearOriginTypeAll = + BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB | + BrowsingDataRemover::ORIGIN_TYPE_PROTECTED_WEB; -// Observes the BrowsingDataRemover that backs the `clearData` method and -// fulfills that API's promise once it's done. This type manages its own -// lifetime, deleting itself once it's done. -class ClearDataObserver : public content::BrowsingDataRemover::Observer { - public: - ClearDataObserver(gin_helper::Promise promise, - content::BrowsingDataRemover* remover) - : promise_(std::move(promise)) { - observation_.Observe(remover); - } +constexpr auto kDataTypeLookup = + base::MakeFixedFlatMap({ + {"backgroundFetch", BrowsingDataRemover::DATA_TYPE_BACKGROUND_FETCH}, + {"cache", BrowsingDataRemover::DATA_TYPE_CACHE | + BrowsingDataRemover::DATA_TYPE_CACHE_STORAGE}, + {"cookies", BrowsingDataRemover::DATA_TYPE_COOKIES}, + {"downloads", BrowsingDataRemover::DATA_TYPE_DOWNLOADS}, + {"fileSystems", BrowsingDataRemover::DATA_TYPE_FILE_SYSTEMS}, + {"indexedDB", BrowsingDataRemover::DATA_TYPE_INDEXED_DB}, + {"localStorage", BrowsingDataRemover::DATA_TYPE_LOCAL_STORAGE}, + {"serviceWorkers", BrowsingDataRemover::DATA_TYPE_SERVICE_WORKERS}, + {"webSQL", BrowsingDataRemover::DATA_TYPE_WEB_SQL}, + }); - void OnBrowsingDataRemoverDone( - content::BrowsingDataRemover::DataType failed_data_types) override { - if (failed_data_types == 0ULL) { - promise_.Resolve(); - } else { - promise_.RejectWithErrorMessage(base::StringPrintf( - "Failed to clear browsing data (%" PRIu64 ")", failed_data_types)); +BrowsingDataRemover::DataType GetDataTypeMask( + const std::vector& data_types) { + BrowsingDataRemover::DataType mask = 0u; + for (const auto& type : data_types) { + if (kDataTypeLookup.contains(type)) { + mask |= kDataTypeLookup.at(type); } - delete this; + } + return mask; +} + +std::vector GetDataTypesFromMask( + BrowsingDataRemover::DataType mask) { + std::vector results; + for (const auto [type, flag] : kDataTypeLookup) { + if (mask & flag) { + results.emplace_back(type); + } + } + return results; +} + +// Represents a task to clear browsing data for the `clearData` API method. +// +// This type manages its own lifetime, deleting itself once the task finishes +// completely. +class ClearDataTask { + public: + // Starts running a task. This function will return before the task is + // finished, but will resolve or reject the |promise| when it finishes. + static void Run( + BrowsingDataRemover* remover, + gin_helper::Promise promise, + BrowsingDataRemover::DataType data_type_mask, + std::vector origins, + BrowsingDataFilterBuilder::Mode filter_mode, + BrowsingDataFilterBuilder::OriginMatchingMode origin_matching_mode) { + std::shared_ptr task(new ClearDataTask(std::move(promise))); + + // This method counts as an operation. This is important so we can call + // `OnOperationFinished` at the end of this method as a fallback if all the + // other operations finished while this method was still executing + task->operations_running_ = 1; + + // Cookies are scoped more broadly than other types of data, so if we are + // filtering then we need to do it at the registrable domain level + if (!origins.empty() && + data_type_mask & BrowsingDataRemover::DATA_TYPE_COOKIES) { + data_type_mask &= ~BrowsingDataRemover::DATA_TYPE_COOKIES; + + auto cookies_filter_builder = + BrowsingDataFilterBuilder::Create(filter_mode); + + for (const url::Origin& origin : origins) { + std::string domain = GetDomainAndRegistry( + origin, + net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES); + if (domain.empty()) { + domain = origin.host(); + } + cookies_filter_builder->AddRegisterableDomain(domain); + } + + StartOperation(task, remover, BrowsingDataRemover::DATA_TYPE_COOKIES, + std::move(cookies_filter_builder)); + } + + // If cookies aren't the only data type and weren't handled above, then we + // can start an operation that is scoped to origins + if (data_type_mask) { + auto filter_builder = + BrowsingDataFilterBuilder::Create(filter_mode, origin_matching_mode); + + for (auto const& origin : origins) { + filter_builder->AddOrigin(origin); + } + + StartOperation(task, remover, data_type_mask, std::move(filter_builder)); + } + + // This static method counts as an operation. + task->OnOperationFinished(std::nullopt); } private: + // An individiual |content::BrowsingDataRemover::Remove...| operation as part + // of a full |ClearDataTask|. This class manages its own lifetime, cleaning + // itself up after the operation completes and notifies the task of the + // result. + class ClearDataOperation : public BrowsingDataRemover::Observer { + public: + static void Run(std::shared_ptr task, + BrowsingDataRemover* remover, + BrowsingDataRemover::DataType data_type_mask, + std::unique_ptr filter_builder) { + auto* operation = new ClearDataOperation(task, remover); + + remover->RemoveWithFilterAndReply(base::Time::Min(), base::Time::Max(), + data_type_mask, kClearOriginTypeAll, + std::move(filter_builder), operation); + } + + // BrowsingDataRemover::Observer: + void OnBrowsingDataRemoverDone( + BrowsingDataRemover::DataType failed_data_types) override { + task_->OnOperationFinished(failed_data_types); + delete this; + } + + private: + ClearDataOperation(std::shared_ptr task, + BrowsingDataRemover* remover) + : task_(task) { + observation_.Observe(remover); + } + + std::shared_ptr task_; + base::ScopedObservation + observation_{this}; + }; + + explicit ClearDataTask(gin_helper::Promise promise) + : promise_(std::move(promise)) {} + + static void StartOperation( + std::shared_ptr task, + BrowsingDataRemover* remover, + BrowsingDataRemover::DataType data_type_mask, + std::unique_ptr filter_builder) { + // Track this operation + task->operations_running_ += 1; + + ClearDataOperation::Run(task, remover, data_type_mask, + std::move(filter_builder)); + } + + void OnOperationFinished( + std::optional failed_data_types) { + DCHECK_GT(operations_running_, 0); + operations_running_ -= 1; + + if (failed_data_types.has_value()) { + failed_data_types_ |= failed_data_types.value(); + } + + // If this is the last operation, then the task is finished + if (operations_running_ == 0) { + OnTaskFinished(); + } + } + + void OnTaskFinished() { + if (failed_data_types_ == 0ULL) { + promise_.Resolve(); + } else { + v8::Isolate* isolate = promise_.isolate(); + + v8::Local failed_data_types_array = + gin::ConvertToV8(isolate, GetDataTypesFromMask(failed_data_types_)); + + // Create a rich error object with extra detail about what data types + // failed + auto error = v8::Exception::Error( + gin::StringToV8(isolate, "Failed to clear data")); + error.As() + ->Set(promise_.GetContext(), + gin::StringToV8(isolate, "failedDataTypes"), + failed_data_types_array) + .Check(); + + promise_.Reject(error); + } + } + + int operations_running_ = 0; + BrowsingDataRemover::DataType failed_data_types_ = 0ULL; gin_helper::Promise promise_; - base::ScopedObservation - observation_{this}; }; base::Value::Dict createProxyConfig(ProxyPrefs::ProxyMode proxy_mode, @@ -1138,17 +1306,85 @@ v8::Local Session::ClearCodeCaches( return handle; } -v8::Local Session::ClearData(gin::Arguments* args) { +v8::Local Session::ClearData(gin_helper::ErrorThrower thrower, + gin::Arguments* args) { auto* isolate = JavascriptEnvironment::GetIsolate(); + + BrowsingDataRemover::DataType data_type_mask = kClearDataTypeAll; + std::vector origins; + BrowsingDataFilterBuilder::OriginMatchingMode origin_matching_mode = + BrowsingDataFilterBuilder::OriginMatchingMode::kThirdPartiesIncluded; + BrowsingDataFilterBuilder::Mode filter_mode = + BrowsingDataFilterBuilder::Mode::kPreserve; + + if (gin_helper::Dictionary options; args->GetNext(&options)) { + if (std::vector data_types; + options.Get("dataTypes", &data_types)) { + data_type_mask = GetDataTypeMask(data_types); + } + + if (bool avoid_closing_connections; + options.Get("avoidClosingConnections", &avoid_closing_connections) && + avoid_closing_connections) { + data_type_mask |= + BrowsingDataRemover::DATA_TYPE_AVOID_CLOSING_CONNECTIONS; + } + + std::vector origin_urls; + { + bool has_origins_key = options.Get("origins", &origin_urls); + std::vector exclude_origin_urls; + bool has_exclude_origins_key = + options.Get("excludeOrigins", &exclude_origin_urls); + + if (has_origins_key && has_exclude_origins_key) { + thrower.ThrowError( + "Cannot provide both 'origins' and 'excludeOrigins'"); + return v8::Undefined(isolate); + } + + if (has_origins_key) { + filter_mode = BrowsingDataFilterBuilder::Mode::kDelete; + } else if (has_exclude_origins_key) { + origin_urls = std::move(exclude_origin_urls); + } + } + + if (!origin_urls.empty()) { + origins.reserve(origin_urls.size()); + for (const GURL& origin_url : origin_urls) { + auto origin = url::Origin::Create(origin_url); + + // Opaque origins cannot be used with this API + if (origin.opaque()) { + thrower.ThrowError( + base::StringPrintf("Invalid origin: '%s'", + origin_url.possibly_invalid_spec().c_str())); + return v8::Undefined(isolate); + } + + origins.push_back(std::move(origin)); + } + } + + if (std::string origin_matching_mode_string; + options.Get("originMatchingMode", &origin_matching_mode_string)) { + if (origin_matching_mode_string == "third-parties-included") { + origin_matching_mode = BrowsingDataFilterBuilder::OriginMatchingMode:: + kThirdPartiesIncluded; + } else if (origin_matching_mode_string == "origin-in-all-contexts") { + origin_matching_mode = + BrowsingDataFilterBuilder::OriginMatchingMode::kOriginInAllContexts; + } + } + } + gin_helper::Promise promise(isolate); v8::Local promise_handle = promise.GetHandle(); - content::BrowsingDataRemover* remover = - browser_context_->GetBrowsingDataRemover(); - - auto* observer = new ClearDataObserver(std::move(promise), remover); - remover->RemoveAndReply(base::Time::Min(), base::Time::Max(), - kClearDataTypeAll, kClearOriginTypeAll, observer); + BrowsingDataRemover* remover = browser_context_->GetBrowsingDataRemover(); + ClearDataTask::Run(remover, std::move(promise), data_type_mask, + std::move(origins), filter_mode, origin_matching_mode); return promise_handle; } diff --git a/shell/browser/api/electron_api_session.h b/shell/browser/api/electron_api_session.h index 70cfdcfcd332..1efe2296fa2e 100644 --- a/shell/browser/api/electron_api_session.h +++ b/shell/browser/api/electron_api_session.h @@ -147,7 +147,8 @@ class Session : public gin::Wrappable, v8::Local GetPath(v8::Isolate* isolate); void SetCodeCachePath(gin::Arguments* args); v8::Local ClearCodeCaches(const gin_helper::Dictionary& options); - v8::Local ClearData(gin::Arguments* args); + v8::Local ClearData(gin_helper::ErrorThrower thrower, + gin::Arguments* args); #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER) base::Value GetSpellCheckerLanguages(); void SetSpellCheckerLanguages(gin_helper::ErrorThrower thrower, diff --git a/spec/api-session-spec.ts b/spec/api-session-spec.ts index 229ac1c5c264..732fae21efed 100644 --- a/spec/api-session-spec.ts +++ b/spec/api-session-spec.ts @@ -1613,7 +1613,7 @@ describe('session module', () => { // NOTE: This API clears more than localStorage, but localStorage is a // convenient test target for this API - it('clears localstorage data', async () => { + it('clears all data when no options supplied', async () => { const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } }); await w.loadFile(path.join(fixtures, 'api', 'localstorage.html')); @@ -1623,7 +1623,8 @@ describe('session module', () => { expect(await w.webContents.executeJavaScript('localStorage.length')).to.equal(0); }); - it('clears localstorage data when called twice in parallel', async () => { + + it('clears all data when no options supplied, called twice in parallel', async () => { const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } }); await w.loadFile(path.join(fixtures, 'api', 'localstorage.html')); @@ -1638,5 +1639,92 @@ describe('session module', () => { // Await the first promise so it doesn't creep into another test await clearDataPromise; }); + + it('only clears specified data categories', async () => { + const w = new BrowserWindow({ + show: false, + webPreferences: { nodeIntegration: true, contextIsolation: false } + }); + await w.loadFile( + path.join(fixtures, 'api', 'localstorage-and-indexeddb.html') + ); + + const { webContents } = w; + const { session } = webContents; + + await once(ipcMain, 'indexeddb-ready'); + + async function queryData (channel: string): Promise { + const event = once(ipcMain, `result-${channel}`); + webContents.send(`get-${channel}`); + return (await event)[1]; + } + + // Data is in localStorage + await expect(queryData('localstorage')).to.eventually.equal('hello localstorage'); + // Data is in indexedDB + await expect(queryData('indexeddb')).to.eventually.equal('hello indexeddb'); + + // Clear only indexedDB, not localStorage + await session.clearData({ dataTypes: ['indexedDB'] }); + + // The localStorage data should still be there + await expect(queryData('localstorage')).to.eventually.equal('hello localstorage'); + + // The indexedDB data should be gone + await expect(queryData('indexeddb')).to.eventually.be.undefined(); + }); + + it('only clears the specified origins', async () => { + const w = new BrowserWindow({ show: false }); + await w.loadURL('about:blank'); + + const { session } = w.webContents; + const { cookies } = session; + + await Promise.all([ + cookies.set({ + url: 'https://example.com/', + name: 'testdotcom', + value: 'testdotcom' + }), + cookies.set({ + url: 'https://example.org/', + name: 'testdotorg', + value: 'testdotorg' + }) + ]); + + await session.clearData({ origins: ['https://example.com'] }); + + expect((await cookies.get({ url: 'https://example.com/', name: 'testdotcom' })).length).to.equal(0); + expect((await cookies.get({ url: 'https://example.org/', name: 'testdotorg' })).length).to.be.greaterThan(0); + }); + + it('clears all except the specified origins', async () => { + const w = new BrowserWindow({ show: false }); + await w.loadURL('about:blank'); + + const { session } = w.webContents; + const { cookies } = session; + + await Promise.all([ + cookies.set({ + url: 'https://example.com/', + name: 'testdotcom', + value: 'testdotcom' + }), + cookies.set({ + url: 'https://example.org/', + name: 'testdotorg', + value: 'testdotorg' + }) + ]); + + await session.clearData({ excludeOrigins: ['https://example.com'] }); + + expect((await cookies.get({ url: 'https://example.com/', name: 'testdotcom' })).length).to.be.greaterThan(0); + expect((await cookies.get({ url: 'https://example.org/', name: 'testdotorg' })).length).to.equal(0); + }); }); }); diff --git a/spec/fixtures/api/localstorage-and-indexeddb.html b/spec/fixtures/api/localstorage-and-indexeddb.html new file mode 100644 index 000000000000..9ba8c3532d2e --- /dev/null +++ b/spec/fixtures/api/localstorage-and-indexeddb.html @@ -0,0 +1,50 @@ + + + + +