Move LoadProgressTracker to the browser process.

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 4cd1943e5e5f12a58bce56a8286f343879532530..c2245a6390a7d0e627e063caeb72846d23c6ec6f 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -340,7 +340,6 @@ class CONTENT_EXPORT WebContentsImpl
   virtual void RenderFrameDeleted(RenderFrameHost* render_frame_host) OVERRIDE;
   virtual void DidStartLoading(RenderFrameHost* render_frame_host,
                                bool to_different_document) OVERRIDE;
-  virtual void DidStopLoading(RenderFrameHost* render_frame_host) OVERRIDE;
   virtual void SwappedOut(RenderFrameHost* render_frame_host) OVERRIDE;
   virtual void WorkerCrashed(RenderFrameHost* render_frame_host) OVERRIDE;
   virtual void ShowContextMenu(RenderFrameHost* render_frame_host,
@@ -389,7 +388,6 @@ class CONTENT_EXPORT WebContentsImpl
   virtual void Close(RenderViewHost* render_view_host) OVERRIDE;
   virtual void RequestMove(const gfx::Rect& new_bounds) OVERRIDE;
   virtual void DidCancelLoading() OVERRIDE;
-  virtual void DidChangeLoadProgress(double progress) OVERRIDE;
   virtual void DocumentAvailableInMainFrame(
       RenderViewHost* render_view_host) OVERRIDE;
   virtual void RouteCloseEvent(RenderViewHost* rvh) OVERRIDE;
@@ -714,6 +712,9 @@ class CONTENT_EXPORT WebContentsImpl
                                const GURL& target_url);
   void OnDocumentLoadedInFrame();
   void OnDidFinishLoad(const GURL& url);
+  void OnDidStartLoading(bool to_different_document);
+  void OnDidStopLoading();
+  void OnDidChangeLoadProgress(double load_progress);
   void OnGoToEntryAtOffset(int offset);
   void OnUpdateZoomLimits(int minimum_percent,
                           int maximum_percent,
@@ -834,6 +835,17 @@ class CONTENT_EXPORT WebContentsImpl
   // called once as this call also removes it from the internal map.
   WebContentsImpl* GetCreatedWindow(int route_id);
 
+  // Tracking loading progress -------------------------------------------------
+
+  // Resets the tracking state of the current load.
+  void ResetLoadProgressState();
+
+  // Calculates the progress of the current load and notifies the delegate.
+  void SendLoadProgressChanged();
+
+  // Called once when the last frame on the page has stopped loading.
+  void DidStopLoading(RenderFrameHost* render_frame_host);
+
   // Misc non-view stuff -------------------------------------------------------
 
   // Helper functions for sending notifications.
@@ -964,6 +976,21 @@ class CONTENT_EXPORT WebContentsImpl
   // The current load state and the URL associated with it.
   net::LoadStateWithParam load_state_;
   base::string16 load_state_host_;
+
+  // LoadingProgressMap maps FrameTreeNode IDs to a double representing that
+  // frame's completion (from 0 to 1).
+  typedef base::hash_map<int64, double> LoadingProgressMap;

[nasko, 2014/05/05 23:51:12]

I think this is the right choice, but one note here. When doing a cross-process
navigation, we start in one process, then in another, and if the navigation will
commit we stop the first process and let the new one finish the navigation.
I know we see didStartProvisionalLoad from both processes, not sure about
didStartLoading. If that's the case, we will overwrite the map entry.

[Avi (use Gerrit), 2014/05/06 00:03:40]

I am not experienced enough in the loading code to know the details of how
things flow here. If you have advice I'd be happy to take it.

[Nate Chapin, 2014/05/07 20:16:02]

Unless we create a way to notify the new process that it is a continuation of a
load that was initiated in a different process, then yes, we will receive
didStartLoading() from both processes. I would think that it's straightforward
to account for that case in WebContentsImpl?

[Charlie Reis, 2014/05/07 23:51:00]

Yes, I don't think it should cause a big problem.  Maybe progress will go
backwards during the swap (depending on whether we start reporting progress
before commit), but we'll have the same FrameTreeNode ID after the swap.

Would make sense to cover this case in a browser test, though, to make sure it
doesn't hit any unexpected checks.

[Nate Chapin, 2014/05/08 00:19:45]

We should be able to avoid going backwards in the continuation case. Currently,
we set a minimum completion of 0.1 at load start, and I don't think we increase
it only when data is received (which should mean no sooner than commit time,
when we're in the correct process).

[Avi (use Gerrit), 2014/05/13 21:08:08]

**THIS ISSUE REMAINS UNADDRESSED**

In the continuation case, does the old frame indicate that its loading has
ended? If so, this shouldn't be horrible. OTOH, if it doesn't, then things will
wait forever.

You refer to a browser test. I need one for this case. Is there a good way to
construct one? We need a good way to churn those out, so that we can test that
case in key places.

[Charlie Reis, 2014/05/13 22:44:37]

We may need to try this out to be sure, since I'm not sure if we'll always hear
a stop from the old frame.  I think transfers should start in one process, issue
a DidFail + DidStop, and then be taken over by the new process. 
Browser-initiated cross-process navigations take a different path, and Nasko
thinks that may have an extra DidStart, but he's checking.  This is all just
speculation on my part, though.
Yes, I think WebContentsImplBrowserTest has a set of LoadingStateChanged tests. 
Nick may be able to help you with setting one up for cross-process subframes if
we want to exercise that path.

[Avi (use Gerrit), 2014/05/14 00:14:32]

This already does a good job of failing just about every CrossSiteTransferTest
and half the SitePerProcessBrowserTests. Can I rely on those?

[Charlie Reis, 2014/05/14 17:18:33]

Yeah, the SitePerProcessBrowserTests exercise the subframe loading case and
should be sufficient.  Note that there's a TODO in NavigateIframeToURL hacking
around this issue: we're listening for a commit instead of load stop, because of
the duplicate load start/stops.

(I think Nasko's new NavigateFrameToURL utility method also does this.)

I mention this because it's fine if we end up hiding these duplicate start/stops
and make it possible for tests to listen for load stop again.  Not sure if
that'll happen here, though.

+  LoadingProgressMap loading_progresses_;
+  double loading_total_progress_;
+
+  base::TimeTicks loading_last_progress_update_;
+
+  base::WeakPtrFactory<WebContentsImpl> loading_weak_factory_;
+
+  // Counter to track how many frames have sent start notifications but not
+  // stop notifications.
+  int loading_frames_in_progress_;
+
   // Upload progress, for displaying in the status bar.
   // Set to zero when there is no significant upload happening.
   uint64 upload_size_;