Reland #2 Remove the is_loading_ field from WebContentsImpl

content/browser/web_contents/web_contents_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_WEB_CONTENTS_WEB_CONTENTS_IMPL_H_
 #define CONTENT_BROWSER_WEB_CONTENTS_WEB_CONTENTS_IMPL_H_
 
 #include <stdint.h>
 
 #include <map>
@@ skipping 689 matching lines @@
   void RenderFrameForInterstitialPageCreated(
       RenderFrameHost* render_frame_host) override;
 
   // Sets the passed interstitial as the currently showing interstitial.
   // No interstitial page should already be attached.
   void AttachInterstitialPage(InterstitialPageImpl* interstitial_page) override;
 
   // 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;
 
   // Forces overscroll to be disabled (used by touch emulation).
   void SetForceDisableOverscrollContent(bool force_disable);
 
   AudioStreamMonitor* audio_stream_monitor() {
     return &audio_stream_monitor_;
   }
 
@@ skipping 284 matching lines @@
   // |preferred_size_for_capture_| changes, to propagate the new value to the
   // |delegate_|.
   void OnPreferredSizeChanged(const gfx::Size& old_size);
 
   // Internal helper to create WebUI objects associated with |this|. |url| is
   // used to determine which WebUI should be created (if any). |frame_name|
   // corresponds to the name of a frame that the WebUI should be created for (or
   // 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.
   WebContentsDelegate* delegate_;
 
   // Handles the back/forward list and loading.
   NavigationControllerImpl controller_;
 
   // The corresponding view.
   scoped_ptr<WebContentsView> view_;
@@ skipping 37 matching lines @@
 
   // If this WebContents is part of a "tree of WebContents", then this contains
   // information about the structure.
   scoped_ptr<WebContentsTreeNode> node_;
 
   // SavePackage, lazily created.
   scoped_refptr<SavePackage> save_package_;
 
   // 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_;
 
   // Indicates if the tab is considered crashed.
   base::TerminationStatus crashed_status_;
   int crashed_error_code_;
 
   // Whether this WebContents is waiting for a first-response for the
   // main resource of the page. This controls whether the throbber state is
@@ skipping 13 matching lines @@
 
   // Upload progress, for displaying in the status bar.
   // Set to zero when there is no significant upload happening.
   uint64_t upload_size_;
   uint64_t upload_position_;
 
   // Tracks that this WebContents needs to unblock requests to the renderer.
   // 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.
   base::string16 page_title_when_no_navigation_entry_;
 
   // When a navigation occurs, we record its contents MIME type. It can be
   // used to check whether we can do something for some special contents.
   std::string contents_mime_type_;
 
   // The last reported character encoding, not canonicalized.
@@ skipping 204 matching lines @@
   // Adds/removes a callback called on creation of each new WebContents.
   static void AddCreatedCallbackForTesting(const CreatedCallback& callback);
   static void RemoveCreatedCallbackForTesting(const CreatedCallback& callback);
 
   DISALLOW_COPY_AND_ASSIGN(FriendZone);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_WEB_CONTENTS_WEB_CONTENTS_IMPL_H_