blimp: Update page load status update indicator to use first paint.

blimp/engine/session/page_load_tracker.h

+// Copyright 2016 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 BLIMP_ENGINE_SESSION_PAGE_LOAD_TRACKER_H_
+#define BLIMP_ENGINE_SESSION_PAGE_LOAD_TRACKER_H_
+
+#include "base/containers/small_map.h"
+#include "base/macros.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_widget_host.h"
+
+namespace blimp {
+namespace engine {
+
+class PageLoadTrackerClient {
+ public:
+  virtual ~PageLoadTrackerClient() {}
+
+  // Called when a navigation status update should be sent to the client.
+  // |load_status| indicates the status of the navigation. An update with a
+  // false value is sent each time a new navigation is initiated. This update
+  // will be followed by a true value when the navigation either successfully
+  // finishes or fails.

[Wez, 2016/05/24 01:42:17]

nit: Feels like this should be an enum w/ values LOADING and LOADED, for
clarity!

[Khushal, 2016/05/24 18:28:10]

Done.

+  virtual void SendPageLoadStatusUpdate(bool load_status) = 0;
+};
+
+// Tracks the page load status for a tab using load and paint notifications
+// from the renderer.
+class PageLoadTracker {
+ public:
+  explicit PageLoadTracker(PageLoadTrackerClient* client);
+  ~PageLoadTracker();
+
+  void DidStartProvisionalLoadForFrame(
+      content::RenderFrameHost* render_frame_host);
+
+  void DidFinishLoad(content::RenderFrameHost* render_frame_host);
+
+  void DidFailLoad(content::RenderFrameHost* render_frame_host);
+
+  void DidFirstPaintAfterLoad(content::RenderWidgetHost* render_widget_host);

[Wez, 2016/05/24 01:42:17]

nit: No need for blank lines between these.

Suggest preceding them with a one-line comment explaining that they mirror calls
in WebContentsObserver, or just make this class a WebContentsObserver?

Is having multiple WebContentsObservers expensive? It does seem to be a very
broad interface...

[Khushal, 2016/05/24 18:28:09]

You're right, having this class as a WebContentsObserver makes purpose of these
calls clearer. Done.

+
+ private:
+  struct LoadStatus {
+    // Set to true on receiving a notification from the renderer that the load
+    // finished. See WebContentsObserver::DidFinishLoad.
+    bool page_loaded = false;
+
+    // Set to true on receiving a notification from the renderer that the first
+    // paint after a navigation was performed.
+    // See WebContentsObserver::DidFirstPaintAfterLoad.
+    bool did_first_paint = false;
+
+    bool Loaded() const;
+  };
+
+  typedef base::SmallMap<std::map<content::RenderWidgetHost*, LoadStatus>>
+      RenderWidgetLoadStatusMap;
+
+  RenderWidgetLoadStatusMap render_widget_load_status_;
+
+  PageLoadTrackerClient* client_;
+
+  DISALLOW_COPY_AND_ASSIGN(PageLoadTracker);
+};
+
+}  // namespace engine
+}  // namespace blimp
+
+#endif  // BLIMP_ENGINE_SESSION_PAGE_LOAD_TRACKER_H_