Remove the is_loading_ field from WebContentsImpl

content/browser/web_contents/web_contents_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/web_contents/web_contents_impl.h"
 
 #include <stddef.h>
 
 #include <cmath>
 #include <utility>
@@ skipping 211 matching lines @@
 
 void SetAccessibilityModeOnFrame(AccessibilityMode mode,
                                  RenderFrameHost* frame_host) {
   static_cast<RenderFrameHostImpl*>(frame_host)->SetAccessibilityMode(mode);
 }
 
 void ResetAccessibility(RenderFrameHost* rfh) {
   static_cast<RenderFrameHostImpl*>(rfh)->AccessibilityReset();
 }
 
+bool NotifyRenderViewTerminated(RenderViewHostImpl* rvh, FrameTreeNode* ftn) {
+  ftn->RenderViewTerminated(rvh);
+  return true;
+}
+
 }  // namespace
 
 WebContents* WebContents::Create(const WebContents::CreateParams& params) {
   FrameTreeNode* opener_node = nullptr;
   if (params.opener_render_frame_id != MSG_ROUTING_NONE) {
     RenderFrameHostImpl* opener_rfh = RenderFrameHostImpl::FromID(
         params.opener_render_process_id, params.opener_render_frame_id);
     if (opener_rfh)
       opener_node = opener_rfh->frame_tree_node();
   }
@@ skipping 124 matching lines @@
       render_view_host_delegate_view_(NULL),
       created_with_opener_(false),
 #if defined(OS_WIN)
       accessible_parent_(NULL),
 #endif
       frame_tree_(new NavigatorImpl(&controller_, this),
                   this,
                   this,
                   this,
                   this),
-      is_loading_(false),
       is_load_to_different_document_(false),
       crashed_status_(base::TERMINATION_STATUS_STILL_RUNNING),
       crashed_error_code_(0),
       waiting_for_response_(false),
       load_state_(net::LOAD_STATE_IDLE, base::string16()),
       upload_size_(0),
       upload_position_(0),
       is_resume_pending_(false),
+      pause_throbber_for_interstitial_(false),
       displayed_insecure_content_(false),
       has_accessed_initial_document_(false),
       theme_color_(SK_ColorTRANSPARENT),
       last_sent_theme_color_(SK_ColorTRANSPARENT),
       did_first_visually_non_empty_paint_(false),
       capturer_count_(0),
       should_normally_be_visible_(true),
       is_being_destroyed_(false),
       notify_disconnection_(false),
       dialog_manager_(NULL),
       is_showing_before_unload_dialog_(false),
       last_active_time_(base::TimeTicks::Now()),
       closed_by_user_gesture_(false),
       minimum_zoom_percent_(static_cast<int>(kMinimumZoomFactor * 100)),
       maximum_zoom_percent_(static_cast<int>(kMaximumZoomFactor * 100)),
       zoom_scroll_remainder_(0),
       render_view_message_source_(NULL),
       render_frame_message_source_(NULL),
       fullscreen_widget_routing_id_(MSG_ROUTING_NONE),
       fullscreen_widget_had_focus_at_shutdown_(false),
       is_subframe_(false),
       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),
       loading_weak_factory_(this) {
   frame_tree_.SetFrameRemoveListener(
       base::Bind(&WebContentsImpl::OnFrameRemoved,
                  base::Unretained(this)));
   media_web_contents_observer_.reset(new MediaWebContentsObserver(this));
   wake_lock_service_context_.reset(new WakeLockServiceContext(this));
 }
@@ skipping 465 matching lines @@
   renderer_preferences_.user_agent_override = override;
 
   // Send the new override string to the renderer.
   RenderViewHost* host = GetRenderViewHost();
   if (host)
     host->SyncRendererPrefs();
 
   // 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_,
                     UserAgentOverrideSet(override));
 }
 
 const std::string& WebContentsImpl::GetUserAgentOverride() const {
   return renderer_preferences_.user_agent_override;
 }
 
@@ skipping 117 matching lines @@
 
 SiteInstanceImpl* WebContentsImpl::GetPendingSiteInstance() const {
   RenderViewHostImpl* dest_rvh =
       GetRenderManager()->pending_render_view_host() ?
           GetRenderManager()->pending_render_view_host() :
           GetRenderManager()->current_host();
   return dest_rvh->GetSiteInstance();
 }
 
 bool WebContentsImpl::IsLoading() const {
-  return is_loading_;
+  return frame_tree_.IsLoading() && !pause_throbber_for_interstitial_;

[nasko, 2016/01/15 01:19:16]

I still think the page is in loading state, even if it is displaying an
interstitial page. A document has been requested and it hasn't been fully loaded
yet. The fact that it is paused because we chose to ask the user something
doesn't mean that it stopped loading.

[clamy, 2016/01/15 16:47:44]

I'm just matching the current behavior here. When we show the interstitial, we
set is_loading_ in the WebContents to false, so WebContentsImpl::IsLoading will
return false. If I remember correctly, not doing it caused issues with some
browser tests.

 }
 
 bool WebContentsImpl::IsLoadingToDifferentDocument() const {
-  return is_loading_ && is_load_to_different_document_;
+  return IsLoading() && is_load_to_different_document_;
 }
 
 bool WebContentsImpl::IsWaitingForResponse() const {
   return waiting_for_response_ && is_load_to_different_document_;
 }
 
 const net::LoadStateWithParam& WebContentsImpl::GetLoadState() const {
   return load_state_;
 }
 
@@ skipping 1238 matching lines @@
     InterstitialPageImpl* interstitial_page) {
   DCHECK(interstitial_page);
   GetRenderManager()->set_interstitial_page(interstitial_page);
 
   // Cancel any visible dialogs so that they don't interfere with the
   // interstitial.
   CancelActiveAndPendingDialogs();
 
   FOR_EACH_OBSERVER(WebContentsObserver, observers_,
                     DidAttachInterstitialPage());
+
+  // Stop the throbber if needed while the interstitial page is shown.
+  if (frame_tree_.IsLoading()) {

[nasko, 2016/01/15 01:19:16]

What if it is already paused?

[clamy, 2016/01/15 16:47:44]

Done.

+    LoadingStateChanged(false, true, NULL);

[nasko, 2016/01/15 01:19:16]

nullptr

[clamy, 2016/01/15 16:47:44]

Done.

+  }
+  pause_throbber_for_interstitial_ = true;

[nasko, 2016/01/15 01:19:16]

If this is state, then it should be "paused".

[clamy, 2016/01/15 16:47:44]

Done.

+}
+
+void WebContentsImpl::DidProceedOnIntertitial() {
+  // Restart the throbber now that the interstitial page is going away.
+  if (pause_throbber_for_interstitial_) {
+    pause_throbber_for_interstitial_ = false;
+    if (frame_tree_.IsLoading())
+      LoadingStateChanged(true, true, NULL);
+  }
 }
 
 void WebContentsImpl::DetachInterstitialPage() {
   if (ShowingInterstitialPage())
     GetRenderManager()->remove_interstitial_page();
   FOR_EACH_OBSERVER(WebContentsObserver, observers_,
                     DidDetachInterstitialPage());
+  // Restart the throbber now that the interstitial page is going away.
+  if (pause_throbber_for_interstitial_) {

[nasko, 2016/01/15 01:19:16]

Looking at the usage here, isn't this equivalent to the value of
ShowingInterstitialPage()?

[clamy, 2016/01/15 16:47:44]

No if the user starts a new navigation, DidProceedOnInterstitial gets called,
the throbber is restarted but the interstitial is still showing. I tried to just
use ShowingInterstitialPage(), but it did not work due to this particular case.

+    pause_throbber_for_interstitial_ = false;
+    if (frame_tree_.IsLoading())
+      LoadingStateChanged(true, true, NULL);
+  }
 }
 
 void WebContentsImpl::SetHistoryOffsetAndLength(int history_offset,
                                                 int history_length) {
   SetHistoryOffsetAndLengthForView(
       GetRenderViewHost(), history_offset, history_length);
 }
 
 void WebContentsImpl::SetHistoryOffsetAndLengthForView(
     RenderViewHost* render_view_host,
@@ skipping 1254 matching lines @@
   if (delegate_)
     delegate_->ShowRepostFormWarningDialog(this);
 }
 
 bool WebContentsImpl::HasAccessedInitialDocument() {
   return has_accessed_initial_document_;
 }
 
 // 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 (pause_throbber_for_interstitial_)
     return;
 
   if (!is_loading) {
     load_state_ = net::LoadStateWithParam(net::LOAD_STATE_IDLE,
                                           base::string16());
     load_state_host_.clear();
     upload_size_ = 0;
     upload_position_ = 0;
   }
 
   GetRenderManager()->SetIsLoading(is_loading);
 
-  is_loading_ = is_loading;
   waiting_for_response_ = is_loading;
   is_load_to_different_document_ = to_different_document;
 
   if (delegate_)
     delegate_->LoadingStateChanged(this, to_different_document);
   NotifyNavigationStateChanged(INVALIDATE_TYPE_LOAD);
 
   std::string url = (details ? details->url.possibly_invalid_spec() : "NULL");
   if (is_loading) {
     TRACE_EVENT_ASYNC_BEGIN2("browser,navigation", "WebContentsImpl Loading",
@@ skipping 332 matching lines @@
   // renderer may not have made a clean exit.
   if (IsFullscreenForCurrentTab(GetRenderViewHost()->GetWidget()))
     ExitFullscreenMode();
 
   // Cancel any visible dialogs so they are not left dangling over the sad tab.
   CancelActiveAndPendingDialogs();
 
   if (delegate_)
     delegate_->HideValidationMessage(this);
 
-  SetIsLoading(false, true, nullptr);
+  frame_tree_.ForEach(base::Bind(&NotifyRenderViewTerminated,
+                                 static_cast<RenderViewHostImpl*>(rvh)));

[nasko, 2016/01/15 01:19:16]

This method (RenderViewTerminated) is called when the process has died. There
are already enough hooks in RFHM and elsewhere that is notified of this event,
so I don't think we need yet more ways to notify hosts of the process dying.

[clamy, 2016/01/15 16:47:44]

Done.

   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();
-
   FOR_EACH_OBSERVER(WebContentsObserver,
                     observers_,
                     RenderProcessGone(GetCrashedStatus()));
 }
 
 void WebContentsImpl::RenderViewDeleted(RenderViewHost* rvh) {
   FOR_EACH_OBSERVER(WebContentsObserver, observers_, RenderViewDeleted(rvh));
 }
 
 void WebContentsImpl::UpdateState(RenderViewHost* rvh,
@@ skipping 72 matching lines @@
     delegate_->SwappedOut(this);
 }
 
 void WebContentsImpl::RequestMove(const gfx::Rect& new_bounds) {
   if (delegate_ && delegate_->IsPopupOrPanel(this))
     delegate_->MoveContents(this, 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.
   //
   // TODO(dmazzoni): do this using a WebContentsObserver.
   BrowserAccessibilityManager* manager =
       frame_tree_node->current_frame_host()->browser_accessibility_manager();
   if (manager)
     manager->UserIsNavigatingAway();
 }
@@ skipping 13 matching lines @@
         base::TimeTicks::Now() - navigator->GetCurrentLoadStart();
 
     details.reset(new LoadNotificationDetails(
         entry->GetVirtualURL(),
         entry->GetTransitionType(),
         elapsed,
         &controller_,
         controller_.GetCurrentEntryIndex()));
   }
 
-  SetIsLoading(false, true, details.get());
+  LoadingStateChanged(false, true, details.get());
 }
 
 void WebContentsImpl::DidChangeLoadProgress() {
   double load_progress = frame_tree_.load_progress();
 
   // The delegate is notified immediately for the first and last updates. Also,
   // since the message loop may be pretty busy when a page is loaded, it might
   // not execute a posted task in a timely manner so the progress report is sent
   // immediately if enough time has passed.
   base::TimeDelta min_delay =
@@ skipping 516 matching lines @@
                                      int render_frame_id,
                                      IPC::Message* reply_msg,
                                      bool dialog_was_suppressed,
                                      bool success,
                                      const base::string16& user_input) {
   RenderFrameHostImpl* rfh = RenderFrameHostImpl::FromID(render_process_id,
                                                          render_frame_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_,
                       BeforeUnloadDialogCancelled());
   }
 
   is_showing_before_unload_dialog_ = false;
   if (rfh) {
     rfh->JavaScriptDialogClosed(reply_msg, success, user_input,
                                 dialog_was_suppressed);
@@ skipping 105 matching lines @@
     const WebContentsObserver::MediaPlayerId& id) {
   FOR_EACH_OBSERVER(WebContentsObserver, observers_, MediaStartedPlaying(id));
 }
 
 void WebContentsImpl::MediaStoppedPlaying(
     const WebContentsObserver::MediaPlayerId& id) {
   FOR_EACH_OBSERVER(WebContentsObserver, observers_, MediaStoppedPlaying(id));
 }
 
 }  // namespace content