Commit 5dc1fe74 authored by Carlos Caballero's avatar Carlos Caballero Committed by Commit Bot

[bfcache] Do not cache if we might show the app banner

Do not cache if we might decide to show the app banner as we would lose
state (e.g. the mojo binding for the render to send the prompt event).
If we cache the page and go back we will not restart the pipeline (no
DidFinishLoad will trigger) that should be fine as we will only put in
cache if no banner needed to be shown. The page will only be cached for
a limited time so we can assume that nothing has happened (e.g. page
suddenly got a manifest) that would change the outcome of the pipeline.

Some background on why we sometimes need to disable bfcache:
https://docs.google.com/document/d/1NjZeusdS1kyEkZyfLggndU1A6qVt0Y1sa-LRUxnMoK8

Bug: 1001087
Change-Id: Id127a52f2883b771f4dbf7311c48683695261449
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824108Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Commit-Queue: Carlos Caballero <carlscab@google.com>
Cr-Commit-Position: refs/heads/master@{#700651}
parent 8aa77f29
......@@ -23,6 +23,7 @@
#include "chrome/common/chrome_switches.h"
#include "components/rappor/public/rappor_utils.h"
#include "components/rappor/rappor_service_impl.h"
#include "content/public/browser/back_forward_cache.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
......@@ -554,6 +555,21 @@ void AppBannerManager::DidFinishNavigation(content::NavigationHandle* handle) {
handle->IsSameDocument()) {
return;
}
// If the page gets stored in the back-forward cache we will not trigger the
// pipeline again when navigating back (DidFinishLoad will not trigger). So
// only allow the page to enter the cache if we know for sure that no
// installation is needed.
// Note: this check must happen before calling Terminate as it might set the
// installable_web_app_check_result_ to kNo.
if (installable_web_app_check_result_ != InstallableWebAppCheckResult::kNo) {
web_contents()
->GetController()
.GetBackForwardCache()
.DisableForRenderFrameHost(handle->GetPreviousRenderFrameHostId(),
"banners::AppBannerManager");
}
if (state_ != State::COMPLETE && state_ != State::INACTIVE)
Terminate();
ResetCurrentPageData();
......
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