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 356 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),
+      paused_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)));
 #if defined(OS_ANDROID)
   media_web_contents_observer_.reset(new MediaWebContentsObserverAndroid(this));
 #else
@@ skipping 469 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() && !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 {
   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 (IsLoading()) {

[nasko, 2016/01/16 00:13:51]

No need for {} in one line if statements.

[clamy, 2016/01/19 13:31:18]

Done.

+    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, NULL);

[nasko, 2016/01/16 00:13:51]

nullptr in all new code.

[clamy, 2016/01/19 13:31:18]

Done.

+  }
 }
 
 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 (paused_throbber_for_interstitial_) {
+    paused_throbber_for_interstitial_ = false;
+    if (frame_tree_.IsLoading())
+      LoadingStateChanged(true, true, NULL);

[nasko, 2016/01/16 00:13:51]

nullptr in all new code.

[clamy, 2016/01/19 13:31:18]

Done.

+  }
 }
 
 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 (paused_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);
   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();

[nasko, 2016/01/16 00:13:51]

I wonder if this is the right place to dispatch this from. The IPC that
BeforeUnload has been completed is handled by RFH itself. Can't we just reset
the state there instead of having to come all the way out to WebContents and go
down back to the RFH?

[clamy, 2016/01/19 13:31:18]

As explained in the other comments about BeforeUnload canceled, there are test
that check explicitly for that behavior. Do we want to modify it? (it would be
user-visible)

     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