Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into master.lion
Browse files Browse the repository at this point in the history
  • Loading branch information
blueboxd committed Feb 21, 2023
2 parents a0f9b32 + 1fa6a1f commit f70e1b6
Show file tree
Hide file tree
Showing 9 changed files with 661 additions and 231 deletions.
2 changes: 1 addition & 1 deletion DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ vars = {
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling freetype
# and whatever else without interference from each other.
'freetype_revision': '713580f41dcc2054d7fa56265f03eeb9c67e0937',
'freetype_revision': '7f9499044e3baa901de99251a007aa66e750b26c',
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling freetype
# and whatever else without interference from each other.
Expand Down
642 changes: 490 additions & 152 deletions content/browser/back_forward_cache_no_store_browsertest.cc

Large diffs are not rendered by default.

47 changes: 4 additions & 43 deletions content/browser/renderer_host/back_forward_cache_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -482,12 +482,7 @@ BackForwardCacheImpl::GetChannelAssociatedMessageHandlingPolicy() {
}

BackForwardCacheImpl::Entry::Entry(std::unique_ptr<StoredPage> stored_page)
: stored_page_(std::move(stored_page)) {
if (BackForwardCacheImpl::AllowStoringPagesWithCacheControlNoStore()) {
cookie_modified_ = {/*http_only_cookie_modified*/ false,
/*cookie_modified*/ false};
}
}
: stored_page_(std::move(stored_page)) {}

BackForwardCacheImpl::Entry::~Entry() = default;

Expand All @@ -497,26 +492,6 @@ void BackForwardCacheImpl::Entry::WriteIntoTrace(
dict.Add("render_frame_host", render_frame_host());
}

void BackForwardCacheImpl::Entry::StartMonitoringCookieChange() {
RenderFrameHostImpl* rfh = stored_page_->render_frame_host();
StoragePartition* storage_partition = rfh->GetStoragePartition();
auto* cookie_manager = storage_partition->GetCookieManagerForBrowserProcess();
if (!cookie_listener_receiver_.is_bound()) {
// Listening only to the main document's URL, not the documents inside the
// subframes.
cookie_manager->AddCookieChangeListener(
rfh->GetLastCommittedURL(), absl::nullopt,
cookie_listener_receiver_.BindNewPipeAndPassRemote());
}
}

void BackForwardCacheImpl::Entry::OnCookieChange(
const net::CookieChangeInfo& change) {
DCHECK(cookie_modified_.has_value());
cookie_modified_->http_only_cookie_modified = change.cookie.IsHttpOnly();
cookie_modified_->cookie_modified = true;
}

void BackForwardCacheImpl::RenderProcessBackgroundedChanged(
RenderProcessHostImpl* host) {
EnforceCacheSizeLimit();
Expand Down Expand Up @@ -630,20 +605,13 @@ void BackForwardCacheImpl::UpdateCanStoreToIncludeCacheControlNoStore(
return;
}

auto* matching_entry = FindMatchingEntry(render_frame_host->GetPage());
// |matching_entry| can be nullptr for tests because this can be called from
// |GetCurrentBackForwardCacheEligibility()|, at which point |rfh| may not
// have a matching entry yet.
if (!matching_entry)
return;

// Note that kCacheControlNoStoreHTTPOnlyCookieModified,
// kCacheControlNoStoreCookieModified and kCacheControlNoStore are mutually
// exclusive.
if (matching_entry->cookie_modified_->http_only_cookie_modified) {
if (render_frame_host->GetCookieChangeInfo().http_only_cookie_modified) {
result.No(BackForwardCacheMetrics::NotRestoredReason::
kCacheControlNoStoreHTTPOnlyCookieModified);
} else if (matching_entry->cookie_modified_->cookie_modified) {
} else if (render_frame_host->GetCookieChangeInfo().cookie_modified) {
// JavaScript cookies are modified but not HTTP cookies. Only restore based
// on the experiment level.
if (GetCacheControlNoStoreLevel() <=
Expand Down Expand Up @@ -1089,14 +1057,6 @@ void BackForwardCacheImpl::StoreEntry(
#endif

entry->render_frame_host()->DidEnterBackForwardCache();
if (AllowStoringPagesWithCacheControlNoStore()) {
if (entry->render_frame_host()->GetBackForwardCacheDisablingFeatures().Has(
WebSchedulerTrackedFeature::kMainResourceHasCacheControlNoStore)) {
// Start monitoring the cookie change only when cache-control:no-store
// header is present.
entry->StartMonitoringCookieChange();
}
}
entry->SetStoredPageDelegate(this);
entries_.push_front(std::move(entry));
AddProcessesForEntry(*entries_.front());
Expand Down Expand Up @@ -1417,6 +1377,7 @@ void BackForwardCacheImpl::WillCommitNavigationToCachedEntry(
}
}

// static
bool BackForwardCacheImpl::AllowStoringPagesWithCacheControlNoStore() {
return GetCacheControlNoStoreLevel() >
CacheControlNoStoreExperimentLevel::kDoNotStore;
Expand Down
33 changes: 6 additions & 27 deletions content/browser/renderer_host/back_forward_cache_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_process_host_observer.h"
#include "content/public/browser/site_instance.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/common/content_features.h"
#include "net/cookies/canonical_cookie.h"
#include "services/network/public/mojom/cookie_manager.mojom.h"
Expand Down Expand Up @@ -139,16 +138,13 @@ class CONTENT_EXPORT BackForwardCacheImpl
GetChannelAssociatedMessageHandlingPolicy();

// BackForwardCache entry, consisting of the page and associated metadata.
class Entry : public ::network::mojom::CookieChangeListener {
class Entry {
public:
explicit Entry(std::unique_ptr<StoredPage> stored_page);
~Entry() override;
~Entry();

void WriteIntoTrace(perfetto::TracedValue context);

// Starts monitoring the cookie change in this entry.
void StartMonitoringCookieChange();

// Indicates whether or not all the |render_view_hosts| in this entry have
// received the acknowledgement from renderer that it finished running
// handlers.
Expand Down Expand Up @@ -184,23 +180,6 @@ class CONTENT_EXPORT BackForwardCacheImpl
private:
friend class BackForwardCacheImpl;

// ::network::mojom::CookieChangeListener
void OnCookieChange(const net::CookieChangeInfo& change) override;

mojo::Receiver<::network::mojom::CookieChangeListener>
cookie_listener_receiver_{this};

struct CookieModified {
// Indicates whether or not cookie on the bfcache entry has been modified
// while the entry is in bfcache.
bool cookie_modified = false;
// Indicates whether or not HTTPOnly cookie on the bfcache entry
// has been modified while the entry is in bfcache.
bool http_only_cookie_modified = false;
};
// Only populated when |AllowStoringPagesWithCacheControlNoStore()| is true.
absl::optional<CookieModified> cookie_modified_;

std::unique_ptr<StoredPage> stored_page_;
};

Expand Down Expand Up @@ -393,6 +372,10 @@ class CONTENT_EXPORT BackForwardCacheImpl
RenderFrameHostImpl& rfh,
BackForwardCacheCanStoreDocumentResult& eviction_reason);

// Returns true if the flag is on for pages with cache-control:no-store to
// get restored from back/forward cache unless cookies change.
static bool AllowStoringPagesWithCacheControlNoStore();

private:
// Destroys all evicted frames in the BackForwardCache.
void DestroyEvictedFrames();
Expand Down Expand Up @@ -456,10 +439,6 @@ class CONTENT_EXPORT BackForwardCacheImpl
void AddProcessesForEntry(Entry& entry);
void RemoveProcessesForEntry(Entry& entry);

// Returns true if the flag is on for pages with cache-control:no-store to
// get restored from back/forward cache unless cookies change.
static bool AllowStoringPagesWithCacheControlNoStore();

static BlockListedFeatures GetAllowedFeatures(
RequestedFeatures requested_features);
static BlockListedFeatures GetDisallowedFeatures(
Expand Down
66 changes: 60 additions & 6 deletions content/browser/renderer_host/navigation_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "content/browser/renderer_host/navigation_request.h"

#include <memory>
#include <string>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -62,6 +63,7 @@
#include "content/browser/preloading/prerender/prerender_host_registry.h"
#include "content/browser/process_lock.h"
#include "content/browser/reduce_accept_language/reduce_accept_language_utils.h"
#include "content/browser/renderer_host/back_forward_cache_impl.h"
#include "content/browser/renderer_host/cookie_utils.h"
#include "content/browser/renderer_host/debug_urls.h"
#include "content/browser/renderer_host/frame_tree.h"
Expand Down Expand Up @@ -2511,6 +2513,23 @@ void NavigationRequest::SetWaitingForRendererResponse() {
SetState(WAITING_FOR_RENDERER_RESPONSE);
}

bool NavigationRequest::ShouldAddCookieChangeListener() {
// The `CookieChangeListener` will only be set up if all of these are true:
// (1) the navigation's protocol is HTTP(s).
// (2) we allow a document with `Cache-control: no-store` header to
// enter BFCache.
// (3) the navigation is neither a same-document navigation nor a page
// activation, since in these cases, an existing `RenderFrameHost` will be
// used, and it would already have an existing listener, so we should skip the
// initialization.
// (4) the navigation is a primary main frame navigation, as the cookie
// change information will only be used in the inactive document control
// logic.
return BackForwardCacheImpl::AllowStoringPagesWithCacheControlNoStore() &&
!IsPageActivation() && !IsSameDocument() && IsInPrimaryMainFrame() &&
common_params_->url.SchemeIsHTTPOrHTTPS();
}

void NavigationRequest::StartNavigation() {
DCHECK(frame_tree_node_->navigation_request() == this ||
is_synchronous_renderer_commit_);
Expand All @@ -2529,6 +2548,24 @@ void NavigationRequest::StartNavigation() {
frame_tree_node->current_frame_host()->GetSiteInstance();
site_info_ = GetSiteInfoForCommonParamsURL();

// It's important to start listening to the cookie changes before the network
// request of the navigation begins in order to ensure the listener won't miss
// any cookie changes that happen after the network request is sent that
// potentially modify some cookie values that are used in this request.
// The information of cookie modification will be used to determine if the
// document that this navigation will load should be eligible for BFCache.
// The listener eventually will be transferred over to the committed
// `RenderFrameHost`.
if (ShouldAddCookieChangeListener()) {
// The listener should receive the change events of the cookies from the
// the domain of the main-frame navigation url.
// If the navigation gets redirected, it will be reset with the new URL when
// `NavigationRequest::OnRequestRedirected()` is called.
cookie_change_listener_ =
std::make_unique<RenderFrameHostImpl::CookieChangeListener>(
GetStoragePartitionWithCurrentSiteInfo(), common_params_->url);
}

// Compute the redirect chain.
// TODO(clamy): Try to simplify this and have the redirects be part of
// CommonNavigationParams.
Expand Down Expand Up @@ -3033,6 +3070,16 @@ void NavigationRequest::OnRequestRedirected(
commit_params_->redirect_infos.back().new_referrer =
common_params_->referrer->url.spec();

// When the redirection happens, the cookie_change_listener_ should be
// re-initialized if needed.
if (ShouldAddCookieChangeListener()) {
cookie_change_listener_ =
std::make_unique<RenderFrameHostImpl::CookieChangeListener>(
GetStoragePartitionWithCurrentSiteInfo(), common_params_->url);
} else {
cookie_change_listener_.reset();
}

// Check Content Security Policy before the NavigationThrottles run. This
// gives CSP a chance to modify requests that NavigationThrottles would
// otherwise block.
Expand Down Expand Up @@ -4443,12 +4490,7 @@ void NavigationRequest::OnStartChecksComplete(
return;
}

// |site_info_|'s StoragePartitionConfig should refer to the correct
// StoragePartition for this navigation.
BrowserContext* browser_context =
frame_tree_node_->navigator().controller().GetBrowserContext();
StoragePartition* partition = browser_context->GetStoragePartition(
site_info_.storage_partition_config());
StoragePartition* partition = GetStoragePartitionWithCurrentSiteInfo();
DCHECK(partition);

// |loader_| should not exist if the service worker handle
Expand Down Expand Up @@ -4600,6 +4642,9 @@ void NavigationRequest::OnStartChecksComplete(
// Reset the compositor lock before starting the loader.
compositor_lock_.reset();

BrowserContext* browser_context =
frame_tree_node_->navigator().controller().GetBrowserContext();

loader_ = NavigationURLLoader::Create(
browser_context, partition,
std::make_unique<NavigationRequestInfo>(
Expand Down Expand Up @@ -8783,4 +8828,13 @@ void NavigationRequest::CheckSoftNavigationHeuristicsInvariants() {
DCHECK(!frame_tree_node()->IsFencedFrameRoot());
}

StoragePartition* NavigationRequest::GetStoragePartitionWithCurrentSiteInfo() {
// `site_info_`'s StoragePartitionConfig should refer to the correct
// `StoragePartition` for this navigation.
return frame_tree_node_->navigator()
.controller()
.GetBrowserContext()
->GetStoragePartition(site_info_.storage_partition_config());
}

} // namespace content
22 changes: 22 additions & 0 deletions content/browser/renderer_host/navigation_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,11 @@ class CONTENT_EXPORT NavigationRequest
void AddDeferredSubframeNavigationThrottle(
base::WeakPtr<SubframeHistoryNavigationThrottle> throttle);

std::unique_ptr<RenderFrameHostImpl::CookieChangeListener>
TakeCookieChangeListener() {
return std::move(cookie_change_listener_);
}

private:
friend class NavigationRequestTest;

Expand Down Expand Up @@ -1767,6 +1772,13 @@ class CONTENT_EXPORT NavigationRequest
// IsInMainFrame().
void UnblockPendingSubframeNavigationRequestsIfNeeded();

// Returns if we should add/reset the `CookieChangeListener` for the current
// navigation.
bool ShouldAddCookieChangeListener();

// Returns the `StoragePartition` based on the config from the `site_info_`.
StoragePartition* GetStoragePartitionWithCurrentSiteInfo();

// Never null. The pointee node owns this navigation request instance.
FrameTreeNode* const frame_tree_node_;

Expand Down Expand Up @@ -2407,6 +2419,16 @@ class CONTENT_EXPORT NavigationRequest
absl::optional<base::UnguessableToken>
main_frame_same_document_navigation_token_;

// The listener that receives cookie change events and maintains cookie change
// information for the domain of the URL that this `NavigationRequest` is
// navigating to. The listener will observe all the cookie changes starting
// from the navigation/redirection, and it will be moved to the
// `RenderFrameHostImpl` when the navigation is committed and continues
// observing until the destruction of the document.
// See `RenderFrameHostImpl::CookieChangeListener`.
std::unique_ptr<RenderFrameHostImpl::CookieChangeListener>
cookie_change_listener_;

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

Expand Down
32 changes: 32 additions & 0 deletions content/browser/renderer_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12180,6 +12180,11 @@ bool RenderFrameHostImpl::DidCommitNavigationInternal(
}
RecordDocumentCreatedUkmEvent(params->origin, document_ukm_source_id,
ukm_recorder);

// We only replace the `CookieChangeListener` with the one initialized by
// the navigation request when navigating to a new document. Otherwise, the
// existing `CookieChangeListener` will be reused.
cookie_change_listener_ = navigation_request->TakeCookieChangeListener();
}

if (!is_same_document_navigation) {
Expand Down Expand Up @@ -14700,4 +14705,31 @@ net::CookieSettingOverrides RenderFrameHostImpl::GetCookieSettingOverrides() {
return subresource_loader_factories_config.cookie_setting_overrides();
}

RenderFrameHostImpl::CookieChangeListener::CookieChangeListener(
StoragePartition* storage_partition,
GURL& url) {
DCHECK(storage_partition);
auto* cookie_manager = storage_partition->GetCookieManagerForBrowserProcess();
cookie_manager->AddCookieChangeListener(
url, absl::nullopt,
cookie_change_listener_receiver_.BindNewPipeAndPassRemote());
}

RenderFrameHostImpl::CookieChangeListener::~CookieChangeListener() = default;

void RenderFrameHostImpl::CookieChangeListener::OnCookieChange(
const net::CookieChangeInfo& change) {
// TODO (https://crbug.com/1399741): After adding the invalidation signals
// API, we could mark the page as ineligible for BFCache as soon as the cookie
// change event is received.
cookie_change_info_.http_only_cookie_modified |= change.cookie.IsHttpOnly();
cookie_change_info_.cookie_modified = true;
}

RenderFrameHostImpl::CookieChangeListener::CookieChangeInfo
RenderFrameHostImpl::GetCookieChangeInfo() {
return cookie_change_listener_ ? cookie_change_listener_->cookie_change_info()
: CookieChangeListener::CookieChangeInfo{};
}

} // namespace content
Loading

0 comments on commit f70e1b6

Please sign in to comment.