Reland #2 Remove the is_loading_ field from WebContentsImpl

content/browser/frame_host/interstitial_page_impl.cc

 // Copyright 2013 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/frame_host/interstitial_page_impl.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 141 matching lines @@
       enabled_(true),
       action_taken_(NO_ACTION),
       render_view_host_(NULL),
       // TODO(nasko): The InterstitialPageImpl will need to provide its own
       // NavigationControllerImpl to the Navigator, which is separate from
       // the WebContents one, so we can enforce no navigation policy here.
       // While we get the code to a point to do this, pass NULL for it.
       // TODO(creis): We will also need to pass delegates for the RVHM as we
       // start to use it.
       frame_tree_(new InterstitialPageNavigatorImpl(this, controller_),
-                  this, this, this,
+                  this,
+                  this,
+                  this,

[Charlie Reis, 2016/02/19 05:24:54]

Curious, is this a git cl format thing?  In general, you don't need each
parameter on its own line for a call site.

[clamy, 2016/02/19 12:02:05]

Yes it's a git cl format change: maybe it's different in an initialization list?

                   static_cast<WebContentsImpl*>(web_contents)),
       original_child_id_(web_contents->GetRenderProcessHost()->GetID()),
       original_rvh_id_(web_contents->GetRenderViewHost()->GetRoutingID()),
       should_revert_web_contents_title_(false),
-      web_contents_was_loading_(false),
       resource_dispatcher_host_notified_(false),
       rvh_delegate_view_(new InterstitialPageRVHDelegateView(this)),
       create_view_(true),
+      pause_throbber_(false),
       delegate_(delegate),
       weak_ptr_factory_(this) {
   InitInterstitialPageMap();
 }
 
 InterstitialPageImpl::~InterstitialPageImpl() {
 }
 
 void InterstitialPageImpl::Show() {
   if (!enabled())
@@ skipping 313 matching lines @@
   if (!enabled()) {
     DontProceed();
     return;
   }
   if (ui::PageTransitionCoreTypeIs(params.transition,
                                    ui::PAGE_TRANSITION_AUTO_SUBFRAME)) {
     // No need to handle navigate message from iframe in the interstitial page.
     return;
   }
 
+  // The interstitial is not loading anymore so stop the throbber.
+  pause_throbber_ = true;
+
   // The RenderViewHost has loaded its contents, we can show it now.
   if (!controller_->delegate()->IsHidden())
     render_view_host_->GetWidget()->GetView()->Show();
   controller_->delegate()->AttachInterstitialPage(this);
 
   RenderWidgetHostView* rwh_view =
       controller_->delegate()->GetRenderViewHost()->GetWidget()->GetView();
 
   // The RenderViewHost may already have crashed before we even get here.
   if (rwh_view) {
     // If the page has focus, focus the interstitial.
     if (rwh_view->HasFocus())
       Focus();
 
     // Hide the original RVH since we're showing the interstitial instead.
     rwh_view->Hide();
   }
-
-  // Notify the tab we are not loading so the throbber is stopped. It also
-  // causes a WebContentsObserver::DidStopLoading callback that the
-  // AutomationProvider (used by the UI tests) expects to consider a navigation
-  // as complete. Without this, navigating in a UI test to a URL that triggers
-  // an interstitial would hang.
-  web_contents_was_loading_ = controller_->delegate()->IsLoading();
-  controller_->delegate()->SetIsLoading(false, true, NULL);
 }
 
 RendererPreferences InterstitialPageImpl::GetRendererPrefs(
     BrowserContext* browser_context) const {
   delegate_->OverrideRendererPrefs(&renderer_preferences_);
   return renderer_preferences_;
 }
 
 void InterstitialPageImpl::RenderWidgetDeleted(
     RenderWidgetHostImpl* render_widget_host) {
@@ skipping 89 matching lines @@
     return;
 
   if (action_taken_ != NO_ACTION) {
     NOTREACHED();
     return;
   }
   Disable();
   action_taken_ = PROCEED_ACTION;
 
   // Resumes the throbber, if applicable.
-  if (web_contents_was_loading_)
-    controller_->delegate()->SetIsLoading(true, true, NULL);
+  pause_throbber_ = false;
+  controller_->delegate()->DidProceedOnInterstitial();
 
   // If this is a new navigation, the old page is going away, so we cancel any
   // blocked requests for it.  If it is not a new navigation, then it means the
   // interstitial was shown as a result of a resource loading in the page.
   // Since the user wants to proceed, we'll let any blocked request go through.
   if (new_navigation_)
     TakeActionOnResourceDispatcher(CANCEL);
   else
     TakeActionOnResourceDispatcher(RESUME);
 
@@ skipping 288 matching lines @@
 void InterstitialPageImpl::UnderlyingContentObserver::NavigationEntryCommitted(
     const LoadCommittedDetails& load_details) {
   interstitial_->OnNavigatingAwayOrTabClosing();
 }
 
 void InterstitialPageImpl::UnderlyingContentObserver::WebContentsDestroyed() {
   interstitial_->OnNavigatingAwayOrTabClosing();
 }
 
 }  // namespace content