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

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

Issue 1545973002: Remove the is_loading_ field from WebContentsImpl (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Reenabled 2 tests Created 4 years, 11 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 4750b1419f05e69eabf8c5f4a002a46398b810ab..5e9c3f986a2f2a46bd127c152ba519cd14eeddc7 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -358,7 +358,6 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context)
this,
this,
this),
- is_loading_(false),
is_load_to_different_document_(false),
crashed_status_(base::TERMINATION_STATUS_STILL_RUNNING),
crashed_error_code_(0),
@@ -367,6 +366,7 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context)
upload_size_(0),
upload_position_(0),
is_resume_pending_(false),
+ paused_throbber_for_interstitial_(false),
displayed_insecure_content_(false),
has_accessed_initial_document_(false),
theme_color_(SK_ColorTRANSPARENT),
@@ -391,8 +391,8 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context)
force_disable_overscroll_content_(false),
last_dialog_suppressed_(false),
geolocation_service_context_(new GeolocationServiceContext()),
- accessibility_mode_(
- BrowserAccessibilityStateImpl::GetInstance()->accessibility_mode()),
+ accessibility_mode_(BrowserAccessibilityStateImpl::GetInstance()
+ ->accessibility_mode()),
audio_stream_monitor_(this),
virtual_keyboard_requested_(false),
page_scale_factor_is_one_(true),
@@ -887,7 +887,7 @@ void WebContentsImpl::SetUserAgentOverride(const std::string& override) {
// Reload the page if a load is currently in progress to avoid having
// different parts of the page loaded using different user agents.
NavigationEntry* entry = controller_.GetVisibleEntry();
- if (is_loading_ && entry != NULL && entry->GetIsOverridingUserAgent())
+ if (IsLoading() && entry != NULL && entry->GetIsOverridingUserAgent())
controller_.ReloadIgnoringCache(true);
FOR_EACH_OBSERVER(WebContentsObserver, observers_,
@@ -1025,11 +1025,11 @@ SiteInstanceImpl* WebContentsImpl::GetPendingSiteInstance() const {
}
bool WebContentsImpl::IsLoading() const {
- return is_loading_;
+ return frame_tree_.IsLoading() && !paused_throbber_for_interstitial_;
}
bool WebContentsImpl::IsLoadingToDifferentDocument() const {
- return is_loading_ && is_load_to_different_document_;
+ return IsLoading() && is_load_to_different_document_;
}
bool WebContentsImpl::IsWaitingForResponse() const {
@@ -2283,6 +2283,21 @@ void WebContentsImpl::AttachInterstitialPage(
FOR_EACH_OBSERVER(WebContentsObserver, observers_,
DidAttachInterstitialPage());
+
+ // Stop the throbber if needed while the interstitial page is shown.
+ if (IsLoading())
+ LoadingStateChanged(false, true, nullptr);
+
+ paused_throbber_for_interstitial_ = true;
+}
+
+void WebContentsImpl::DidProceedOnInterstitial() {
+ // Restart the throbber now that the interstitial page is going away.
+ if (paused_throbber_for_interstitial_) {
+ paused_throbber_for_interstitial_ = false;
+ if (frame_tree_.IsLoading())
+ LoadingStateChanged(true, true, nullptr);
+ }
}
void WebContentsImpl::DetachInterstitialPage() {
@@ -2290,6 +2305,12 @@ void WebContentsImpl::DetachInterstitialPage() {
GetRenderManager()->remove_interstitial_page();
FOR_EACH_OBSERVER(WebContentsObserver, observers_,
DidDetachInterstitialPage());
+ // Restart the throbber now that the interstitial page is going away.
+ if (paused_throbber_for_interstitial_) {
+ paused_throbber_for_interstitial_ = false;
+ if (frame_tree_.IsLoading())
+ LoadingStateChanged(true, true, nullptr);
+ }
}
void WebContentsImpl::SetHistoryOffsetAndLength(int history_offset,
@@ -3564,10 +3585,11 @@ bool WebContentsImpl::HasAccessedInitialDocument() {
// Notifies the RenderWidgetHost instance about the fact that the page is
// loading, or done loading.
-void WebContentsImpl::SetIsLoading(bool is_loading,
- bool to_different_document,
- LoadNotificationDetails* details) {
- if (is_loading == is_loading_)
+void WebContentsImpl::LoadingStateChanged(bool is_loading,
+ bool to_different_document,
+ LoadNotificationDetails* details) {
+ // Do not send notifications about loading while the interstitial is showing.
+ if (paused_throbber_for_interstitial_)
return;
if (!is_loading) {
@@ -3580,7 +3602,6 @@ void WebContentsImpl::SetIsLoading(bool is_loading,
GetRenderManager()->SetIsLoading(is_loading);
- is_loading_ = is_loading;
waiting_for_response_ = is_loading;
is_load_to_different_document_ = to_different_document;
@@ -3933,15 +3954,13 @@ void WebContentsImpl::RenderViewTerminated(RenderViewHost* rvh,
if (delegate_)
delegate_->HideValidationMessage(this);
- SetIsLoading(false, true, nullptr);
- NotifyDisconnected();
- SetIsCrashed(status, error_code);
-
// Reset the loading progress. TODO(avi): What does it mean to have a
// "renderer crash" when there is more than one renderer process serving a
// webpage? Once this function is called at a more granular frame level, we
// probably will need to more granularly reset the state here.
ResetLoadProgressState();
+ NotifyDisconnected();
+ SetIsCrashed(status, error_code);
FOR_EACH_OBSERVER(WebContentsObserver,
observers_,
@@ -4035,7 +4054,7 @@ void WebContentsImpl::RequestMove(const gfx::Rect& new_bounds) {
void WebContentsImpl::DidStartLoading(FrameTreeNode* frame_tree_node,
bool to_different_document) {
- SetIsLoading(true, to_different_document, nullptr);
+ LoadingStateChanged(true, to_different_document, nullptr);
// Notify accessibility that the user is navigating away from the
// current document.
@@ -4069,7 +4088,7 @@ void WebContentsImpl::DidStopLoading() {
controller_.GetCurrentEntryIndex()));
}
- SetIsLoading(false, true, details.get());
+ LoadingStateChanged(false, true, details.get());
}
void WebContentsImpl::DidChangeLoadProgress() {
@@ -4606,10 +4625,8 @@ void WebContentsImpl::OnDialogClosed(int render_process_id,
last_dialog_suppressed_ = dialog_was_suppressed;
if (is_showing_before_unload_dialog_ && !success) {
- // If a beforeunload dialog is canceled, we need to stop the throbber from
- // spinning, since we forced it to start spinning in Navigate.
if (rfh)
- DidStopLoading();
+ rfh->frame_tree_node()->BeforeUnloadCanceled();
controller_.DiscardNonCommittedEntries();
FOR_EACH_OBSERVER(WebContentsObserver, observers_,

Powered by Google App Engine
This is Rietveld 408576698