Reland #2 Remove the is_loading_ field from WebContentsImpl

content/browser/web_contents/web_contents_impl.h

Index: content/browser/web_contents/web_contents_impl.h
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 75b0bd2b1052f6518bb7f32aa6e4a7647e2f0920..d6825b699003ba64b9a5abc41eb0f36a32faf93e 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -707,12 +707,8 @@ class CONTENT_EXPORT WebContentsImpl
   // Unsets the currently showing interstitial.
   void DetachInterstitialPage() override;
 
-  // Changes the IsLoading state and notifies the delegate as needed.
-  // |details| is used to provide details on the load that just finished
-  // (but can be null if not applicable).
-  void SetIsLoading(bool is_loading,
-                    bool to_different_document,
-                    LoadNotificationDetails* details) override;
+  // Unpause the throbber if it was paused.
+  void DidProceedOnInterstitial() override;
 
   typedef base::Callback<void(WebContents*)> CreatedCallback;
 
@@ -1017,6 +1013,16 @@ class CONTENT_EXPORT WebContentsImpl
   // the main frame if empty).
   WebUI* CreateWebUI(const GURL& url, const std::string& frame_name);
 
+  // Notifies the delegate of a change in loading state.
+  // |details| is used to provide details on the load that just finished
+  // (but can be null if not applicable).
+  // |pause_throbber_for_interstitial_| will be used to update
+  // pause_throbber_for_interstitial_.
+  void LoadingStateChanged(bool is_loading,
+                           bool to_different_document,
+                           bool pause_throbber_for_interstitial,
+                           LoadNotificationDetails* details);
+
   // Data for core operation ---------------------------------------------------
 
   // Delegate for notifying our owner about stuff. Not owned by us.
@@ -1074,9 +1080,6 @@ class CONTENT_EXPORT WebContentsImpl
 
   // Data for loading state ----------------------------------------------------
 
-  // Indicates whether we're currently loading a resource.
-  bool is_loading_;
-
   // Indicates whether the current load is to a different document. Only valid
   // if is_loading_ is true.
   bool is_load_to_different_document_;
@@ -1110,6 +1113,10 @@ class CONTENT_EXPORT WebContentsImpl
   // See ResumeLoadingCreatedWebContents.
   bool is_resume_pending_;
 
+  // Whether the throbber is suspended while an interstial page is showing.
+  // This is set to false when the user proceeds in the interstitial.
+  bool paused_throbber_for_interstitial_;

[Charlie Reis, 2016/02/17 05:15:28]

Again, I'm not sure if this is an improvement.  is_loading_ seems like a more
straightforward concept than this one.

[clamy, 2016/02/17 14:42:06]

Now that I think about it, it should be possible to move this to the
InterstitialPage itself. Then whether the WebContents is loading would be:
FrameTree::IsLoading && !InterstitialPage::PauseThrobber.

[Charlie Reis, 2016/02/17 23:34:26]

Yes, that sounds a bit nicer.

[clamy, 2016/02/18 17:20:26]

Done.

+
   // Data for current page -----------------------------------------------------
 
   // When a title cannot be taken from any entry, this title will be used.