Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(488)

Unified Diff: content/browser/web_contents/web_contents_impl.cc

Issue 925623002: Refactor the loading tracking logic in WebContentsImpl. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix compile Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 3c46724086182f256112f0cb2c3841faf15eb3fa..c1a19460c487d12cc3b9f73be9d343dbdbac9028 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -122,9 +122,6 @@ namespace {
const int kMinimumDelayBetweenLoadingUpdatesMS = 100;
-// This matches what Blink's ProgressTracker has traditionally used for a
-// minimum progress value.
-const double kMinimumLoadingProgress = 0.1;
const char kDotGoogleDotCom[] = ".google.com";
@@ -164,6 +161,26 @@ bool CollectSites(BrowserContext* context,
return true;
}
+// Helper function for retrieving the total loading progress and number of
+// frames in a frame tree.
+bool CollectLoadProgress(double* progress,
+ int* frame_count,
+ FrameTreeNode* node) {
+ *progress += node->get_loading_progress();
+ (*frame_count)++;
+ return true;
+}
+
+// Helper function to check if at least one of the nodes is loading.
+bool IsNodeLoading(bool* is_loading, FrameTreeNode* node) {
+ if (node->is_loading()) {
+ // There is at least one node loading, abort traversal.
+ *is_loading = true;
+ return false;
+ }
+ return true;
+}
+
bool ForEachFrameInternal(
const base::Callback<void(RenderFrameHost*)>& on_frame,
FrameTreeNode* node) {
@@ -334,7 +351,6 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context,
waiting_for_response_(false),
load_state_(net::LOAD_STATE_IDLE, base::string16()),
loading_total_progress_(0.0),
- loading_frames_in_progress_(0),
upload_size_(0),
upload_position_(0),
displayed_insecure_content_(false),
@@ -2831,14 +2847,13 @@ void WebContentsImpl::OnDidStartLoading(bool to_different_document) {
RenderFrameHostImpl* rfh =
static_cast<RenderFrameHostImpl*>(render_frame_message_source_);
- int64 render_frame_id = rfh->frame_tree_node()->frame_tree_node_id();
// Any main frame load to a new document should reset the load progress, since
// it will replace the current page and any frames.
if (to_different_document && !rfh->GetParent()) {
ResetLoadProgressState();
- loading_frames_in_progress_ = 0;
rfh->frame_tree_node()->set_is_loading(false);
+ rfh->frame_tree_node()->set_loading_progress(kNotStartedLoadingProgress);
}
// It is possible to get multiple calls to OnDidStartLoading that don't have
@@ -2856,17 +2871,15 @@ void WebContentsImpl::OnDidStartLoading(bool to_different_document) {
if (rfh->frame_tree_node()->is_loading())
return;
- DCHECK_GE(loading_frames_in_progress_, 0);
- if (loading_frames_in_progress_ == 0)
+ if (!IsFrameTreeLoading())
DidStartLoading(rfh, to_different_document);
- ++loading_frames_in_progress_;
rfh->frame_tree_node()->set_is_loading(true);
+ rfh->frame_tree_node()->set_loading_progress(kMinimumLoadingProgress);
// Notify the RenderFrameHostManager of the event.
rfh->frame_tree_node()->render_manager()->OnDidStartLoading();
- loading_progresses_[render_frame_id] = kMinimumLoadingProgress;
SendLoadProgressChanged();
}
@@ -2876,28 +2889,22 @@ void WebContentsImpl::OnDidStopLoading() {
RenderFrameHostImpl* rfh =
static_cast<RenderFrameHostImpl*>(render_frame_message_source_);
- int64 render_frame_id = rfh->frame_tree_node()->frame_tree_node_id();
+
+ // This method should never be called when the frame is not loading.
+ DCHECK(rfh->frame_tree_node()->is_loading());
rfh->frame_tree_node()->set_is_loading(false);
+ rfh->frame_tree_node()->set_loading_progress(kDoneLoadingProgress);
- if (loading_progresses_.find(render_frame_id) != loading_progresses_.end()) {
- // Load stopped while we were still tracking load. Make sure we update
- // progress based on this frame's completion.
- loading_progresses_[render_frame_id] = 1.0;
- SendLoadProgressChanged();
- // Then we clean-up our states.
- if (loading_total_progress_ == 1.0)
- ResetLoadProgressState();
- }
+ // Update progress based on this frame's completion.
+ SendLoadProgressChanged();
+ // Then clean-up the states.
+ if (loading_total_progress_ == 1.0)
+ ResetLoadProgressState();
// Notify the RenderFrameHostManager of the event.
rfh->frame_tree_node()->render_manager()->OnDidStopLoading();
- // TODO(japhet): This should be a DCHECK, but the pdf plugin sometimes
- // calls DidStopLoading() without a matching DidStartLoading().
- if (loading_frames_in_progress_ == 0)
- return;
- --loading_frames_in_progress_;
- if (loading_frames_in_progress_ == 0)
+ if (!IsFrameTreeLoading())
DidStopLoading(rfh);
}
@@ -2907,9 +2914,8 @@ void WebContentsImpl::OnDidChangeLoadProgress(double load_progress) {
RenderFrameHostImpl* rfh =
static_cast<RenderFrameHostImpl*>(render_frame_message_source_);
- int64 render_frame_id = rfh->frame_tree_node()->frame_tree_node_id();
- loading_progresses_[render_frame_id] = load_progress;
+ rfh->frame_tree_node()->set_loading_progress(load_progress);
// We notify progress change immediately for the first and last updates.
// Also, since the message loop may be pretty busy when a page is loaded, it
@@ -3391,12 +3397,8 @@ void WebContentsImpl::SendLoadProgressChanged() {
double progress = 0.0;
int frame_count = 0;
- for (LoadingProgressMap::iterator it = loading_progresses_.begin();
- it != loading_progresses_.end();
- ++it) {
- progress += it->second;
- ++frame_count;
- }
+ frame_tree_.ForEach(
+ base::Bind(&CollectLoadProgress, &progress, &frame_count));
if (frame_count == 0)
return;
progress /= frame_count;
@@ -3411,7 +3413,6 @@ void WebContentsImpl::SendLoadProgressChanged() {
}
void WebContentsImpl::ResetLoadProgressState() {
- loading_progresses_.clear();
loading_total_progress_ = 0.0;
loading_weak_factory_.InvalidateWeakPtrs();
loading_last_progress_update_ = base::TimeTicks();
@@ -3728,7 +3729,6 @@ void WebContentsImpl::RenderViewTerminated(RenderViewHost* rvh,
// webpage? Once this function is called at a more granular frame level, we
// probably will need to more granularly reset the state here.
ResetLoadProgressState();
- loading_frames_in_progress_ = 0;
FOR_EACH_OBSERVER(WebContentsObserver,
observers_,
@@ -4512,4 +4512,10 @@ void WebContentsImpl::SetForceDisableOverscrollContent(bool force_disable) {
view_->SetOverscrollControllerEnabled(CanOverscrollContent());
}
+bool WebContentsImpl::IsFrameTreeLoading() {
+ bool is_loading = false;
+ frame_tree_.ForEach(base::Bind(&IsNodeLoading, &is_loading));
+ return is_loading;
+}
+
} // namespace content

Powered by Google App Engine
This is Rietveld 408576698