Commit bb7a31d7 authored by Arthur Hemery's avatar Arthur Hemery Committed by Commit Bot

[bfcache] Post-BFCache::Entry nits

Followup CL addressing comments by arthursonzogni@ on:
https://chromium-review.googlesource.com/c/chromium/src/+/1738451

Bug: 990816
Change-Id: Ibc1610c188e3082a049695dcd1c89dd3f710d264
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1832819
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: default avatarArthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701937}
parent b49b1a52
...@@ -128,26 +128,6 @@ BackForwardCacheImpl::Entry::Entry(std::unique_ptr<RenderFrameHostImpl> rfh, ...@@ -128,26 +128,6 @@ BackForwardCacheImpl::Entry::Entry(std::unique_ptr<RenderFrameHostImpl> rfh,
: render_frame_host(std::move(rfh)), proxy_hosts(std::move(proxies)) {} : render_frame_host(std::move(rfh)), proxy_hosts(std::move(proxies)) {}
BackForwardCacheImpl::Entry::~Entry() {} BackForwardCacheImpl::Entry::~Entry() {}
int BackForwardCacheImpl::Entry::GetNavigationEntryId() {
CHECK(render_frame_host);
return render_frame_host->nav_entry_id();
}
bool BackForwardCacheImpl::Entry::IsEvictedFromBackForwardCache() {
CHECK(render_frame_host);
return render_frame_host->is_evicted_from_back_forward_cache();
}
void BackForwardCacheImpl::Entry::EvictFromBackForwardCache() {
CHECK(render_frame_host);
return render_frame_host->EvictFromBackForwardCache();
}
void BackForwardCacheImpl::Entry::LeaveBackForwardCache() {
CHECK(render_frame_host);
return render_frame_host->LeaveBackForwardCache();
}
std::string BackForwardCacheImpl::CanStoreDocumentResult::ToString() { std::string BackForwardCacheImpl::CanStoreDocumentResult::ToString() {
using Reason = BackForwardCacheMetrics::CanNotStoreDocumentReason; using Reason = BackForwardCacheMetrics::CanNotStoreDocumentReason;
...@@ -324,10 +304,10 @@ void BackForwardCacheImpl::StoreEntry( ...@@ -324,10 +304,10 @@ void BackForwardCacheImpl::StoreEntry(
// full. // full.
size_t available_count = 0; size_t available_count = 0;
for (auto& stored_entry : entries_) { for (auto& stored_entry : entries_) {
if (stored_entry->IsEvictedFromBackForwardCache()) if (stored_entry->render_frame_host->is_evicted_from_back_forward_cache())
continue; continue;
if (++available_count > size_limit) if (++available_count > size_limit)
stored_entry->EvictFromBackForwardCache(); stored_entry->render_frame_host->EvictFromBackForwardCache();
} }
} }
...@@ -353,7 +333,7 @@ std::unique_ptr<BackForwardCacheImpl::Entry> BackForwardCacheImpl::RestoreEntry( ...@@ -353,7 +333,7 @@ std::unique_ptr<BackForwardCacheImpl::Entry> BackForwardCacheImpl::RestoreEntry(
auto matching_entry = std::find_if( auto matching_entry = std::find_if(
entries_.begin(), entries_.end(), entries_.begin(), entries_.end(),
[navigation_entry_id](std::unique_ptr<Entry>& entry) { [navigation_entry_id](std::unique_ptr<Entry>& entry) {
return entry->GetNavigationEntryId() == navigation_entry_id; return entry->render_frame_host->nav_entry_id() == navigation_entry_id;
}); });
// Not found. // Not found.
...@@ -361,12 +341,13 @@ std::unique_ptr<BackForwardCacheImpl::Entry> BackForwardCacheImpl::RestoreEntry( ...@@ -361,12 +341,13 @@ std::unique_ptr<BackForwardCacheImpl::Entry> BackForwardCacheImpl::RestoreEntry(
return nullptr; return nullptr;
// Don't restore an evicted frame. // Don't restore an evicted frame.
if ((*matching_entry)->IsEvictedFromBackForwardCache()) if ((*matching_entry)
->render_frame_host->is_evicted_from_back_forward_cache())
return nullptr; return nullptr;
std::unique_ptr<Entry> entry = std::move(*matching_entry); std::unique_ptr<Entry> entry = std::move(*matching_entry);
entries_.erase(matching_entry); entries_.erase(matching_entry);
entry->LeaveBackForwardCache(); entry->render_frame_host->LeaveBackForwardCache();
return entry; return entry;
} }
...@@ -403,14 +384,15 @@ BackForwardCacheImpl::Entry* BackForwardCacheImpl::GetEntry( ...@@ -403,14 +384,15 @@ BackForwardCacheImpl::Entry* BackForwardCacheImpl::GetEntry(
auto matching_entry = std::find_if( auto matching_entry = std::find_if(
entries_.begin(), entries_.end(), entries_.begin(), entries_.end(),
[navigation_entry_id](std::unique_ptr<Entry>& entry) { [navigation_entry_id](std::unique_ptr<Entry>& entry) {
return entry->GetNavigationEntryId() == navigation_entry_id; return entry->render_frame_host->nav_entry_id() == navigation_entry_id;
}); });
if (matching_entry == entries_.end()) if (matching_entry == entries_.end())
return nullptr; return nullptr;
// Don't return the frame if it is evicted. // Don't return the frame if it is evicted.
if ((*matching_entry)->IsEvictedFromBackForwardCache()) if ((*matching_entry)
->render_frame_host->is_evicted_from_back_forward_cache())
return nullptr; return nullptr;
return (*matching_entry).get(); return (*matching_entry).get();
...@@ -420,9 +402,9 @@ void BackForwardCacheImpl::DestroyEvictedFrames() { ...@@ -420,9 +402,9 @@ void BackForwardCacheImpl::DestroyEvictedFrames() {
TRACE_EVENT0("navigation", "BackForwardCache::DestroyEvictedFrames"); TRACE_EVENT0("navigation", "BackForwardCache::DestroyEvictedFrames");
if (entries_.empty()) if (entries_.empty())
return; return;
entries_.erase(std::remove_if(entries_.begin(), entries_.end(), entries_.erase(std::remove_if(
[](std::unique_ptr<Entry>& entry) { entries_.begin(), entries_.end(), [](std::unique_ptr<Entry>& entry) {
return entry->IsEvictedFromBackForwardCache(); return entry->render_frame_host->is_evicted_from_back_forward_cache();
})); }));
} }
} // namespace content } // namespace content
...@@ -43,14 +43,6 @@ class CONTENT_EXPORT BackForwardCacheImpl : public BackForwardCache { ...@@ -43,14 +43,6 @@ class CONTENT_EXPORT BackForwardCacheImpl : public BackForwardCache {
RenderFrameProxyHostMap proxy_hosts); RenderFrameProxyHostMap proxy_hosts);
~Entry(); ~Entry();
// These functions forward to the underlying render_frame_host. Do not call
// just before storing or right after restoring, as the Entry will be in an
// invalid state.
int GetNavigationEntryId();
bool IsEvictedFromBackForwardCache();
void EvictFromBackForwardCache();
void LeaveBackForwardCache();
// The main document being stored. // The main document being stored.
std::unique_ptr<RenderFrameHostImpl> render_frame_host; std::unique_ptr<RenderFrameHostImpl> render_frame_host;
......
...@@ -2308,7 +2308,7 @@ void NavigationRequest::CommitNavigation() { ...@@ -2308,7 +2308,7 @@ void NavigationRequest::CommitNavigation() {
std::unique_ptr<BackForwardCacheImpl::Entry> restored_bfcache_entry = std::unique_ptr<BackForwardCacheImpl::Entry> restored_bfcache_entry =
controller->GetBackForwardCache().RestoreEntry(nav_entry_id_); controller->GetBackForwardCache().RestoreEntry(nav_entry_id_);
// The only time restored_rfh can be nullptr here, is if the // The only time restored_bfcache_entry can be nullptr here, is if the
// document was evicted from the BackForwardCache since this navigation // document was evicted from the BackForwardCache since this navigation
// started. // started.
// //
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment