Remove the is_loading_ field from WebContentsImpl

content/browser/web_contents/web_contents_impl.cc

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..80c21cd9e8bcc29c66d4e6c04f6601cea3252e77 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,16 +3954,9 @@ 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();

[nasko, 2016/01/20 20:58:34]

Why was this removed?

[clamy, 2016/01/22 13:08:15]

Brought it back.

-
   FOR_EACH_OBSERVER(WebContentsObserver,
                     observers_,
                     RenderProcessGone(GetCrashedStatus()));
@@ -4035,7 +4049,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 +4083,7 @@ void WebContentsImpl::DidStopLoading() {
         controller_.GetCurrentEntryIndex()));
   }
 
-  SetIsLoading(false, true, details.get());
+  LoadingStateChanged(false, true, details.get());
 }
 
 void WebContentsImpl::DidChangeLoadProgress() {
@@ -4606,10 +4620,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_,