Prerendering: Fire WebContentsObserver::LoadProgressChanged only on activation
This CL delays firing LoadProgressChanged for prerender frame tree and
dispatches the notification only on activation during calls to
DidStartLoading and DidStopLoading which invoked on activation.
Why?
This is done to ensure that WebContentsObserver::LoadProgressChanged is
per-WebContents i.e., it is not affected by navigations in the
prerendering frame tree.
Now the observers don't use LoadProgressChanged to dispatch show UI
visible changes and other events on an inactive RenderFrameHost.
Document explaining load events and prerendering:
https://docs.google.com/document/d/1WqzPSpWtJ9bqaaecYWTWvg6h2Q1UMh7kVB8C61o8QL4/edit?usp=sharing&resourcekey=0-q9YIhsh5LS-ZfBXcBQCOew
Bug: 1199682
Change-Id: I638d319802a6b6e77748af4057835dc47461fa5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2993855
Commit-Queue: Sreeja Kamishetty <sreejakshetty@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#911364}
diff --git a/content/browser/renderer_host/frame_tree.cc b/content/browser/renderer_host/frame_tree.cc
index 7f96b53f..3cf1577 100644
--- a/content/browser/renderer_host/frame_tree.cc
+++ b/content/browser/renderer_host/frame_tree.cc
@@ -23,6 +23,7 @@
#include "content/browser/renderer_host/navigation_request.h"
#include "content/browser/renderer_host/navigator.h"
#include "content/browser/renderer_host/navigator_delegate.h"
+#include "content/browser/renderer_host/page_impl.h"
#include "content/browser/renderer_host/render_frame_host_delegate.h"
#include "content/browser/renderer_host/render_frame_host_factory.h"
#include "content/browser/renderer_host/render_frame_host_impl.h"
@@ -33,6 +34,7 @@
#include "content/common/content_switches_internal.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/common/frame/frame_policy.h"
+#include "third_party/blink/public/common/loader/loader_constants.h"
#include "third_party/blink/public/mojom/frame/frame_owner_properties.mojom.h"
namespace content {
@@ -561,8 +563,11 @@
focused_frame_tree_node_id_ = FrameTreeNode::kFrameTreeNodeInvalidId;
}
-void FrameTree::ResetLoadProgress() {
- load_progress_ = 0.0;
+double FrameTree::GetLoadProgress() {
+ if (root_->HasNavigation())
+ return blink::kInitialLoadProgress;
+
+ return root_->current_frame_host()->GetPage().load_progress();
}
bool FrameTree::IsLoading() const {
@@ -672,12 +677,6 @@
void FrameTree::DidStartLoadingNode(FrameTreeNode& node,
bool to_different_document,
bool was_previously_loading) {
- // Any main frame load to a new document should reset the load progress since
- // it will replace the current page and any frames. The WebContents will
- // be notified when DidChangeLoadProgress is called.
- if (to_different_document && node.IsMainFrame())
- ResetLoadProgress();
-
if (was_previously_loading)
return;
@@ -693,18 +692,6 @@
delegate_->DidStopLoading();
}
-void FrameTree::DidChangeLoadProgressForNode(FrameTreeNode& node,
- double load_progress) {
- if (!node.IsMainFrame())
- return;
- if (load_progress <= load_progress_)
- return;
- load_progress_ = load_progress;
-
- // Notify the WebContents.
- delegate_->DidChangeLoadProgress();
-}
-
void FrameTree::DidCancelLoading() {
OPTIONAL_TRACE_EVENT0("content", "FrameTree::DidCancelLoading");
navigator_.controller().DiscardNonCommittedEntries();
diff --git a/content/browser/renderer_host/frame_tree.h b/content/browser/renderer_host/frame_tree.h
index 556d2df46..c249c92 100644
--- a/content/browser/renderer_host/frame_tree.h
+++ b/content/browser/renderer_host/frame_tree.h
@@ -355,14 +355,11 @@
bool to_different_document,
bool was_previously_loading);
void DidStopLoadingNode(FrameTreeNode& node);
- void DidChangeLoadProgressForNode(FrameTreeNode& node, double load_progress);
void DidCancelLoading();
- // Returns this FrameTree's total load progress.
- double load_progress() const { return load_progress_; }
-
- // Resets the load progress on all nodes in this FrameTree.
- void ResetLoadProgress();
+ // Returns this FrameTree's total load progress. If the `root_` FrameTreeNode
+ // is navigating returns `blink::kInitialLoadProgress`.
+ double GetLoadProgress();
// Returns true if at least one of the nodes in this FrameTree is loading.
bool IsLoading() const;
diff --git a/content/browser/renderer_host/frame_tree_node.cc b/content/browser/renderer_host/frame_tree_node.cc
index 7596d07..99e01371 100644
--- a/content/browser/renderer_host/frame_tree_node.cc
+++ b/content/browser/renderer_host/frame_tree_node.cc
@@ -28,6 +28,7 @@
#include "services/network/public/cpp/web_sandbox_flags.h"
#include "services/network/public/mojom/web_sandbox_flags.mojom-shared.h"
#include "third_party/blink/public/common/features.h"
+#include "third_party/blink/public/common/loader/loader_constants.h"
#include "third_party/blink/public/mojom/frame/user_activation_update_types.mojom.h"
#include "third_party/blink/public/mojom/security_context/insecure_request_policy.mojom.h"
@@ -42,12 +43,6 @@
base::LazyInstance<FrameTreeNodeIdMap>::DestructorAtExit
g_frame_tree_node_id_map = LAZY_INSTANCE_INITIALIZER;
-// These values indicate the loading progress status. The minimum progress
-// value matches what Blink's ProgressTracker has traditionally used for a
-// minimum progress value.
-const double kLoadingProgressMinimum = 0.1;
-const double kLoadingProgressDone = 1.0;
-
} // namespace
// This observer watches the opener of its owner FrameTreeNode and clears the
@@ -568,7 +563,7 @@
// Set initial load progress and update overall progress. This will notify
// the WebContents of the load progress change.
- DidChangeLoadProgress(kLoadingProgressMinimum);
+ DidChangeLoadProgress(blink::kInitialLoadProgress);
// Notify the RenderFrameHostManager of the event.
render_manager()->OnDidStartLoading();
@@ -579,7 +574,7 @@
frame_tree_node_id());
// Set final load progress and update overall progress. This will notify
// the WebContents of the load progress change.
- DidChangeLoadProgress(kLoadingProgressDone);
+ DidChangeLoadProgress(blink::kFinalLoadProgress);
// Notify the RenderFrameHostManager of the event.
render_manager()->OnDidStopLoading();
@@ -588,9 +583,9 @@
}
void FrameTreeNode::DidChangeLoadProgress(double load_progress) {
- DCHECK_GE(load_progress, kLoadingProgressMinimum);
- DCHECK_LE(load_progress, kLoadingProgressDone);
- frame_tree_->DidChangeLoadProgressForNode(*this, load_progress);
+ DCHECK_GE(load_progress, blink::kInitialLoadProgress);
+ DCHECK_LE(load_progress, blink::kFinalLoadProgress);
+ current_frame_host()->DidChangeLoadProgress(load_progress);
}
bool FrameTreeNode::StopLoading() {
diff --git a/content/browser/renderer_host/page_impl.cc b/content/browser/renderer_host/page_impl.cc
index bfedc35..18fbbeb 100644
--- a/content/browser/renderer_host/page_impl.cc
+++ b/content/browser/renderer_host/page_impl.cc
@@ -13,6 +13,7 @@
#include "content/browser/renderer_host/render_view_host_delegate.h"
#include "content/browser/renderer_host/render_view_host_impl.h"
#include "content/public/browser/render_view_host.h"
+#include "third_party/blink/public/common/loader/loader_constants.h"
#include "third_party/perfetto/include/perfetto/tracing/traced_value.h"
namespace content {
@@ -140,6 +141,13 @@
void PageImpl::MaybeDispatchLoadEventsOnPrerenderActivation() {
DCHECK(IsPrimary());
+ // Dispatch LoadProgressChanged notification on activation with the
+ // prerender last load progress value if the value is not equal to
+ // blink::kFinalLoadProgress, whose notification is dispatched during call
+ // to DidStopLoading.
+ if (load_progress() != blink::kFinalLoadProgress)
+ main_document_.DidChangeLoadProgress(load_progress());
+
main_document_.ForEachRenderFrameHost(
base::BindRepeating([](RenderFrameHostImpl* rfh) {
rfh->MaybeDispatchDOMContentLoadedOnPrerenderActivation();
diff --git a/content/browser/renderer_host/page_impl.h b/content/browser/renderer_host/page_impl.h
index 5a221ea6..444194b 100644
--- a/content/browser/renderer_host/page_impl.h
+++ b/content/browser/renderer_host/page_impl.h
@@ -114,6 +114,11 @@
// activation.
void MaybeDispatchLoadEventsOnPrerenderActivation();
+ void set_load_progress(double load_progress) {
+ load_progress_ = load_progress;
+ }
+ double load_progress() const { return load_progress_; }
+
private:
void DidActivateAllRenderViewsForPrerendering();
@@ -126,6 +131,10 @@
// run for the main document.
bool is_on_load_completed_in_main_document_ = false;
+ // Overall load progress of this Page. Initial load progress value is 0.0
+ // before the load has begun.
+ double load_progress_ = 0.0;
+
// Web application manifest URL for this page.
// See https://w3c.github.io/manifest/#web-application-manifest.
//
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index d38584b..0cfabc4 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -5353,7 +5353,15 @@
}
void RenderFrameHostImpl::DidChangeLoadProgress(double load_progress) {
- frame_tree_node_->DidChangeLoadProgress(load_progress);
+ if (!is_main_frame())
+ return;
+
+ if (load_progress < GetPage().load_progress())
+ return;
+
+ GetPage().set_load_progress(load_progress);
+
+ frame_tree_node_->frame_tree()->delegate()->DidChangeLoadProgress();
}
void RenderFrameHostImpl::DidFinishLoad(const GURL& validated_url) {
