Skip to content

Commit

Permalink
Merge pull request #8526 from brave/cf-crash-2
Browse files Browse the repository at this point in the history 
enable ephemeral storage keep alive by default
  • Loading branch information
@bridiver
bridiver authored Apr 22, 2021
2 parents 9387d52 + 508c5b8 commit e446573
Show file tree
Hide file tree
Showing 10 changed files with 235 additions and 57 deletions.
1 change: 1 addition & 0 deletions browser/ephemeral_storage/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ if (!is_android) {
]
defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
deps = [
":ephemeral_storage",
"//base",
"//brave/components/brave_shields/browser:browser",
"//brave/components/brave_shields/common:common",
Expand Down
134 changes: 108 additions & 26 deletions browser/ephemeral_storage/ephemeral_storage_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
#include <string>

#include "base/path_service.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "base/time/time.h"
#include "brave/browser/ephemeral_storage/ephemeral_storage_tab_helper.h"
#include "brave/common/brave_paths.h"
#include "brave/components/brave_shields/browser/brave_shields_util.h"
#include "brave/components/brave_shields/common/brave_shield_constants.h"
Expand Down Expand Up @@ -33,6 +36,8 @@ using net::test_server::EmbeddedTestServer;

namespace {

const int kKeepAliveInterval = 2;

enum StorageType { Session, Local };

const char* ToString(StorageType storage_type) {
Expand Down Expand Up @@ -75,10 +80,7 @@ content::EvalJsResult GetCookiesInFrame(RenderFrameHost* host) {
class EphemeralStorageBrowserTest : public InProcessBrowserTest {
public:
EphemeralStorageBrowserTest()
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
scoped_feature_list_.InitAndEnableFeature(
net::features::kBraveEphemeralStorage);
}
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}

void SetUpOnMainThread() override {
InProcessBrowserTest::SetUpOnMainThread();
Expand All @@ -100,6 +102,10 @@ class EphemeralStorageBrowserTest : public InProcessBrowserTest {
https_server_.GetURL("b.com", "/ephemeral_storage.html");
c_site_ephemeral_storage_url_ =
https_server_.GetURL("c.com", "/ephemeral_storage.html");

ephemeral_storage::EphemeralStorageTabHelper::
SetKeepAliveTimeDelayForTesting(
base::TimeDelta::FromSeconds(kKeepAliveInterval));
}

void SetUpCommandLine(base::CommandLine* command_line) override {
Expand Down Expand Up @@ -167,7 +173,6 @@ class EphemeralStorageBrowserTest : public InProcessBrowserTest {

protected:
net::test_server::EmbeddedTestServer https_server_;
base::test::ScopedFeatureList scoped_feature_list_;
GURL a_site_ephemeral_storage_url_;
GURL b_site_ephemeral_storage_url_;
GURL c_site_ephemeral_storage_url_;
Expand Down Expand Up @@ -253,41 +258,67 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, StorageIsPartitioned) {
}

IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest,
NavigatingClearsEphemeralStorage) {
NavigatingClearsEphemeralStorageAfterKeepAlive) {
ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_);
auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents();

SetValuesInFrames(web_contents, "a.com value", "from=a.com");

ValuesFromFrames values_before = GetValuesFromFrames(web_contents);
EXPECT_EQ("a.com value", values_before.main_frame.local_storage);
EXPECT_EQ("a.com value", values_before.iframe_1.local_storage);
EXPECT_EQ("a.com value", values_before.iframe_2.local_storage);
ValuesFromFrames values = GetValuesFromFrames(web_contents);
EXPECT_EQ("a.com value", values.main_frame.local_storage);
EXPECT_EQ("a.com value", values.iframe_1.local_storage);
EXPECT_EQ("a.com value", values.iframe_2.local_storage);

EXPECT_EQ("a.com value", values_before.main_frame.session_storage);
EXPECT_EQ("a.com value", values_before.iframe_1.session_storage);
EXPECT_EQ("a.com value", values_before.iframe_2.session_storage);
EXPECT_EQ("a.com value", values.main_frame.session_storage);
EXPECT_EQ("a.com value", values.iframe_1.session_storage);
EXPECT_EQ("a.com value", values.iframe_2.session_storage);

EXPECT_EQ("from=a.com", values_before.main_frame.cookies);
EXPECT_EQ("from=a.com", values_before.iframe_1.cookies);
EXPECT_EQ("from=a.com", values_before.iframe_2.cookies);
EXPECT_EQ("from=a.com", values.main_frame.cookies);
EXPECT_EQ("from=a.com", values.iframe_1.cookies);
EXPECT_EQ("from=a.com", values.iframe_2.cookies);

// Navigate away and then navigate back to the original site.
ui_test_utils::NavigateToURL(browser(), b_site_ephemeral_storage_url_);
ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_);

ValuesFromFrames values_after = GetValuesFromFrames(web_contents);
EXPECT_EQ("a.com value", values_after.main_frame.local_storage);
EXPECT_EQ(nullptr, values_after.iframe_1.local_storage);
EXPECT_EQ(nullptr, values_after.iframe_2.local_storage);
// within keepalive values should be the same
ValuesFromFrames before_timeout = GetValuesFromFrames(web_contents);
EXPECT_EQ("a.com value", before_timeout.main_frame.local_storage);
EXPECT_EQ("a.com value", before_timeout.iframe_1.local_storage);
EXPECT_EQ("a.com value", before_timeout.iframe_2.local_storage);

EXPECT_EQ("a.com value", values_after.main_frame.session_storage);
EXPECT_EQ(nullptr, values_after.iframe_1.session_storage);
EXPECT_EQ(nullptr, values_after.iframe_2.session_storage);
// keepalive does not apply to session storage
EXPECT_EQ("a.com value", before_timeout.main_frame.session_storage);
EXPECT_EQ(nullptr, before_timeout.iframe_1.session_storage);
EXPECT_EQ(nullptr, before_timeout.iframe_2.session_storage);

EXPECT_EQ("from=a.com", values_after.main_frame.cookies);
EXPECT_EQ("", values_after.iframe_1.cookies);
EXPECT_EQ("", values_after.iframe_2.cookies);
EXPECT_EQ("from=a.com", before_timeout.main_frame.cookies);
EXPECT_EQ("from=a.com", before_timeout.iframe_1.cookies);
EXPECT_EQ("from=a.com", before_timeout.iframe_2.cookies);

// after keepalive values should be cleared
ui_test_utils::NavigateToURL(browser(), b_site_ephemeral_storage_url_);

base::RunLoop run_loop;
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, run_loop.QuitClosure(),
base::TimeDelta::FromSeconds(kKeepAliveInterval));
run_loop.Run();

ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_);

ValuesFromFrames after_timeout = GetValuesFromFrames(web_contents);
EXPECT_EQ("a.com value", after_timeout.main_frame.local_storage);
EXPECT_EQ(nullptr, after_timeout.iframe_1.local_storage);
EXPECT_EQ(nullptr, after_timeout.iframe_2.local_storage);

EXPECT_EQ("a.com value", after_timeout.main_frame.session_storage);
EXPECT_EQ(nullptr, after_timeout.iframe_1.session_storage);
EXPECT_EQ(nullptr, after_timeout.iframe_2.session_storage);

EXPECT_EQ("from=a.com", after_timeout.main_frame.cookies);
EXPECT_EQ("", after_timeout.iframe_1.cookies);
EXPECT_EQ("", after_timeout.iframe_2.cookies);
}

IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest,
Expand Down Expand Up @@ -533,3 +564,54 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest,
EXPECT_EQ("third-party-a.com", third_party_values.session_storage);
EXPECT_EQ("name=third-party-a.com", third_party_values.cookies);
}

class EphemeralStorageKeepAliveDisabledBrowserTest
: public EphemeralStorageBrowserTest {
public:
EphemeralStorageKeepAliveDisabledBrowserTest()
: EphemeralStorageBrowserTest() {
scoped_feature_list_.InitAndDisableFeature(
net::features::kBraveEphemeralStorageKeepAlive);
}

private:
base::test::ScopedFeatureList scoped_feature_list_;
};

IN_PROC_BROWSER_TEST_F(EphemeralStorageKeepAliveDisabledBrowserTest,
NavigatingClearsEphemeralStorageWhenKeepAliveDisabled) {
ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_);
auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents();

SetValuesInFrames(web_contents, "a.com value", "from=a.com");

ValuesFromFrames values_before = GetValuesFromFrames(web_contents);
EXPECT_EQ("a.com value", values_before.main_frame.local_storage);
EXPECT_EQ("a.com value", values_before.iframe_1.local_storage);
EXPECT_EQ("a.com value", values_before.iframe_2.local_storage);

EXPECT_EQ("a.com value", values_before.main_frame.session_storage);
EXPECT_EQ("a.com value", values_before.iframe_1.session_storage);
EXPECT_EQ("a.com value", values_before.iframe_2.session_storage);

EXPECT_EQ("from=a.com", values_before.main_frame.cookies);
EXPECT_EQ("from=a.com", values_before.iframe_1.cookies);
EXPECT_EQ("from=a.com", values_before.iframe_2.cookies);

// Navigate away and then navigate back to the original site.
ui_test_utils::NavigateToURL(browser(), b_site_ephemeral_storage_url_);
ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_);

ValuesFromFrames values_after = GetValuesFromFrames(web_contents);
EXPECT_EQ("a.com value", values_after.main_frame.local_storage);
EXPECT_EQ(nullptr, values_after.iframe_1.local_storage);
EXPECT_EQ(nullptr, values_after.iframe_2.local_storage);

EXPECT_EQ("a.com value", values_after.main_frame.session_storage);
EXPECT_EQ(nullptr, values_after.iframe_1.session_storage);
EXPECT_EQ(nullptr, values_after.iframe_2.session_storage);

EXPECT_EQ("from=a.com", values_after.main_frame.cookies);
EXPECT_EQ("", values_after.iframe_1.cookies);
EXPECT_EQ("", values_after.iframe_2.cookies);
}
69 changes: 50 additions & 19 deletions browser/ephemeral_storage/ephemeral_storage_tab_helper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@

#include "base/feature_list.h"
#include "base/hash/md5.h"
#include "base/no_destructor.h"
#include "base/optional.h"
#include "base/ranges/ranges.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/session_storage_namespace.h"
Expand All @@ -33,8 +32,6 @@ namespace {
constexpr char kSessionStorageSuffix[] = "/ephemeral-session-storage";
constexpr char kLocalStorageSuffix[] = "/ephemeral-local-storage";

const base::TimeDelta kStorageKeepAliveDelay = base::TimeDelta::FromSeconds(30);

base::TimeDelta g_storage_keep_alive_for_testing = base::TimeDelta::Min();

// Session storage ids are expected to be 36 character long GUID strings. Since
Expand Down Expand Up @@ -68,6 +65,11 @@ EphemeralStorageTabHelper::EphemeralStorageTabHelper(WebContents* web_contents)

EphemeralStorageTabHelper::~EphemeralStorageTabHelper() {}

void EphemeralStorageTabHelper::WebContentsDestroyed() {
keep_alive_tld_ephemeral_lifetime_list_.clear();
keep_alive_local_storage_list_.clear();
}

void EphemeralStorageTabHelper::ReadyToCommitNavigation(
NavigationHandle* navigation_handle) {
if (!navigation_handle->IsInMainFrame())
Expand All @@ -76,6 +78,7 @@ void EphemeralStorageTabHelper::ReadyToCommitNavigation(
return;

const GURL& new_url = navigation_handle->GetURL();

std::string new_domain = net::URLToEphemeralStorageDomain(new_url);
std::string previous_domain =
net::URLToEphemeralStorageDomain(web_contents()->GetLastCommittedURL());
Expand All @@ -85,6 +88,28 @@ void EphemeralStorageTabHelper::ReadyToCommitNavigation(
CreateEphemeralStorageAreasForDomainAndURL(new_domain, new_url);
}

void EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive(
const content::TLDEphemeralLifetimeKey& key) {
ClearLocalStorageKeepAlive(
StringToSessionStorageId(key.second, kLocalStorageSuffix));

auto it = base::ranges::find_if(keep_alive_tld_ephemeral_lifetime_list_,
[&key](const auto& tld_ephermal_liftime) {
return tld_ephermal_liftime->key() == key;
});
if (it != keep_alive_tld_ephemeral_lifetime_list_.end())
keep_alive_tld_ephemeral_lifetime_list_.erase(it);
}

void EphemeralStorageTabHelper::ClearLocalStorageKeepAlive(
const std::string& id) {
auto it = base::ranges::find_if(
keep_alive_local_storage_list_,
[&id](const auto& local_storage) { return local_storage->id() == id; });
if (it != keep_alive_local_storage_list_.end())
keep_alive_local_storage_list_.erase(it);
}

void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL(
const std::string& new_domain,
const GURL& new_url) {
Expand All @@ -97,6 +122,27 @@ void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL(
auto* partition =
BrowserContext::GetStoragePartition(browser_context, site_instance.get());

if (base::FeatureList::IsEnabled(
net::features::kBraveEphemeralStorageKeepAlive) &&
tld_ephemeral_lifetime_) {
keep_alive_tld_ephemeral_lifetime_list_.push_back(tld_ephemeral_lifetime_);
keep_alive_local_storage_list_.push_back(local_storage_namespace_);

// keep the ephemeral storage alive for some time to handle redirects
// including meta refresh or other page driven "redirects" that end up back
// at the original origin
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(
&EphemeralStorageTabHelper::ClearEphemeralLifetimeKeepalive,
weak_factory_.GetWeakPtr(), tld_ephemeral_lifetime_->key()),
g_storage_keep_alive_for_testing.is_min()
? base::TimeDelta::FromSeconds(
net::features::kBraveEphemeralStorageKeepAliveTimeInSeconds
.Get())
: g_storage_keep_alive_for_testing);
}

// This will fetch a session storage namespace for this storage partition
// and storage domain. If another tab helper is already using the same
// namespace, this will just give us a new reference. When the last tab helper
Expand Down Expand Up @@ -129,21 +175,6 @@ void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL(
kSessionStorageSuffix))
: base::nullopt);

if (base::FeatureList::IsEnabled(
net::features::kBraveEphemeralStorageKeepAlive)) {
// keep the ephemeral storage alive for some time to handle redirects
// including meta refresh or other page driven "redirects" that end up back
// at the original origin
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce([](scoped_refptr<content::TLDEphemeralLifetime>
tld_ephemeral_lifetime) {},
tld_ephemeral_lifetime_),
g_storage_keep_alive_for_testing.is_min()
? kStorageKeepAliveDelay
: g_storage_keep_alive_for_testing);
}

tld_ephemeral_lifetime_ = content::TLDEphemeralLifetime::GetOrCreate(
browser_context, partition, new_domain);
}
Expand Down
18 changes: 16 additions & 2 deletions browser/ephemeral_storage/ephemeral_storage_tab_helper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@

#include <string>
#include <utility>
#include <vector>

#include "base/memory/weak_ptr.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/session_storage_namespace.h"
#include "content/public/browser/web_contents_observer.h"
Expand All @@ -35,11 +37,16 @@ class EphemeralStorageTabHelper

static void SetKeepAliveTimeDelayForTesting(const base::TimeDelta& time);

protected:
private:
// WebContentsObserver
void ReadyToCommitNavigation(
content::NavigationHandle* navigation_handle) override;
void WebContentsDestroyed() override;

void ClearEphemeralLifetimeKeepalive(
const content::TLDEphemeralLifetimeKey& key);
void ClearLocalStorageKeepAlive(const std::string& id);

private:
void CreateEphemeralStorageAreasForDomainAndURL(const std::string& new_domain,
const GURL& new_url);

Expand All @@ -48,6 +55,13 @@ class EphemeralStorageTabHelper
scoped_refptr<content::SessionStorageNamespace> session_storage_namespace_;
scoped_refptr<content::TLDEphemeralLifetime> tld_ephemeral_lifetime_;

std::vector<scoped_refptr<content::TLDEphemeralLifetime>>
keep_alive_tld_ephemeral_lifetime_list_;
std::vector<scoped_refptr<content::SessionStorageNamespace>>
keep_alive_local_storage_list_;

base::WeakPtrFactory<EphemeralStorageTabHelper> weak_factory_{this};

WEB_CONTENTS_USER_DATA_KEY_DECL();
};

Expand Down
1 change: 1 addition & 0 deletions browser/permissions/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ if (!is_android) {

deps = [
"//brave/browser:browser_process",
"//brave/browser/ephemeral_storage",
"//chrome/browser",
"//chrome/browser/ui",
"//chrome/test:test_support",
Expand Down
Loading

0 comments on commit e446573

Please sign in to comment.