Rework and fix loadVisuallyCommitted signal

- Remove RenderViewObserverHostQt_DidFirstVisuallyNonEmptyLayout message
and use WebContentsObserver::DidFirstVisuallyNonEmptyPaint instead.
- The order of the DidFirstVisuallyNonEmptyPaint and
OnSwapCompositorFrame events is not guaranteed. Check for both events to
do not miss to send any loadVisuallyCommitted signal.

Change-Id: Ic733b3e9a6fae64b1d827e8e07514f180273cf8c
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

8 files changed, 31 insertions, 26 deletions

diff --git a/src/core/render_view_observer_host_qt.cpp b/src/core/render_view_observer_host_qt.cpp
index c03cecb38..643eba007 100644
--- a/src/core/render_view_observer_host_qt.cpp
+++ b/src/core/render_view_observer_host_qt.cpp
@@ -71,8 +71,6 @@ bool RenderViewObserverHostQt::OnMessageReceived(const IPC::Message& message)
                             onDidFetchDocumentMarkup)
         IPC_MESSAGE_HANDLER(RenderViewObserverHostQt_DidFetchDocumentInnerText,
                             onDidFetchDocumentInnerText)
-        IPC_MESSAGE_HANDLER(RenderViewObserverHostQt_DidFirstVisuallyNonEmptyLayout,
-                            onDidFirstVisuallyNonEmptyLayout)
         IPC_MESSAGE_UNHANDLED(handled = false)
     IPC_END_MESSAGE_MAP()
     return handled;
@@ -89,11 +87,4 @@ void RenderViewObserverHostQt::onDidFetchDocumentInnerText(quint64 requestId, co
     m_adapterClient->didFetchDocumentInnerText(requestId, toQt(innerText));
 }
 
-void RenderViewObserverHostQt::onDidFirstVisuallyNonEmptyLayout()
-{
-    RenderWidgetHostViewQt *rwhv = static_cast<RenderWidgetHostViewQt*>(web_contents()->GetRenderWidgetHostView());
-    if (rwhv)
-        rwhv->didFirstVisuallyNonEmptyLayout();
-}
-
 } // namespace QtWebEngineCore
diff --git a/src/core/render_view_observer_host_qt.h b/src/core/render_view_observer_host_qt.h
index f352be7b9..42d232b01 100644
--- a/src/core/render_view_observer_host_qt.h
+++ b/src/core/render_view_observer_host_qt.h
@@ -63,7 +63,6 @@ private:
     bool OnMessageReceived(const IPC::Message& message) Q_DECL_OVERRIDE;
     void onDidFetchDocumentMarkup(quint64 requestId, const base::string16& markup);
     void onDidFetchDocumentInnerText(quint64 requestId, const base::string16& innerText);
-    void onDidFirstVisuallyNonEmptyLayout();
 
     WebContentsAdapterClient *m_adapterClient;
 };
diff --git a/src/core/render_widget_host_view_qt.cpp b/src/core/render_widget_host_view_qt.cpp
index 097dda1cd..70ffe0dcb 100644
--- a/src/core/render_widget_host_view_qt.cpp
+++ b/src/core/render_widget_host_view_qt.cpp
@@ -236,7 +236,7 @@ RenderWidgetHostViewQt::RenderWidgetHostViewQt(content::RenderWidgetHost* widget
     , m_touchMotionStarted(false)
     , m_chromiumCompositorData(new ChromiumCompositorData)
     , m_needsDelegatedFrameAck(false)
-    , m_didFirstVisuallyNonEmptyLayout(false)
+    , m_loadVisuallyCommittedState(NotCommitted)
     , m_adapterClient(0)
     , m_imeInProgress(false)
     , m_receivedEmptyImeText(false)
@@ -676,9 +676,11 @@ void RenderWidgetHostViewQt::OnSwapCompositorFrame(uint32_t output_surface_id, c
 
     m_delegate->update();
 
-    if (m_didFirstVisuallyNonEmptyLayout) {
+    if (m_loadVisuallyCommittedState == NotCommitted) {
+        m_loadVisuallyCommittedState = DidFirstCompositorFrameSwap;
+    } else if (m_loadVisuallyCommittedState == DidFirstVisuallyNonEmptyPaint) {
         m_adapterClient->loadVisuallyCommitted();
-        m_didFirstVisuallyNonEmptyLayout = false;
+        m_loadVisuallyCommittedState = NotCommitted;
     }
 
     if (scrollOffsetChanged)
@@ -1244,9 +1246,4 @@ void RenderWidgetHostViewQt::handleFocusEvent(QFocusEvent *ev)
     }
 }
 
-void RenderWidgetHostViewQt::didFirstVisuallyNonEmptyLayout()
-{
-    m_didFirstVisuallyNonEmptyLayout = true;
-}
-
 } // namespace QtWebEngineCore
diff --git a/src/core/render_widget_host_view_qt.h b/src/core/render_widget_host_view_qt.h
index d6c77fada..0b2d7bc9d 100644
--- a/src/core/render_widget_host_view_qt.h
+++ b/src/core/render_widget_host_view_qt.h
@@ -104,6 +104,12 @@ class RenderWidgetHostViewQt
 #endif // QT_NO_ACCESSIBILITY
 {
 public:
+    enum LoadVisuallyCommittedState {
+        NotCommitted,
+        DidFirstVisuallyNonEmptyPaint,
+        DidFirstCompositorFrameSwap
+    };
+
     RenderWidgetHostViewQt(content::RenderWidgetHost* widget);
     ~RenderWidgetHostViewQt();
 
@@ -193,7 +199,8 @@ public:
 #ifndef QT_NO_ACCESSIBILITY
     virtual void accessibilityActiveChanged(bool active) Q_DECL_OVERRIDE;
 #endif // QT_NO_ACCESSIBILITY
-    void didFirstVisuallyNonEmptyLayout();
+    LoadVisuallyCommittedState getLoadVisuallyCommittedState() const { return m_loadVisuallyCommittedState; }
+    void setLoadVisuallyCommittedState(LoadVisuallyCommittedState state) { m_loadVisuallyCommittedState = state; }
 
     gfx::SizeF lastContentsSize() const { return m_lastContentsSize; }
 
@@ -218,7 +225,7 @@ private:
     QExplicitlySharedDataPointer<ChromiumCompositorData> m_chromiumCompositorData;
     cc::ReturnedResourceArray m_resourcesToRelease;
     bool m_needsDelegatedFrameAck;
-    bool m_didFirstVisuallyNonEmptyLayout;
+    LoadVisuallyCommittedState m_loadVisuallyCommittedState;
     uint32_t m_pendingOutputSurfaceId;
 
     QMetaObject::Connection m_adapterClientDestroyedConnection;
diff --git a/src/core/renderer/render_view_observer_qt.cpp b/src/core/renderer/render_view_observer_qt.cpp
index 393b4752c..97485afad 100644
--- a/src/core/renderer/render_view_observer_qt.cpp
+++ b/src/core/renderer/render_view_observer_qt.cpp
@@ -83,11 +83,6 @@ void RenderViewObserverQt::onSetBackgroundColor(quint32 color)
     render_view()->GetWebFrameWidget()->setBaseBackgroundColor(color);
 }
 
-void RenderViewObserverQt::OnFirstVisuallyNonEmptyLayout()
-{
-    Send(new RenderViewObserverHostQt_DidFirstVisuallyNonEmptyLayout(routing_id()));
-}
-
 bool RenderViewObserverQt::OnMessageReceived(const IPC::Message& message)
 {
     bool handled = true;
diff --git a/src/core/renderer/render_view_observer_qt.h b/src/core/renderer/render_view_observer_qt.h
index b62c815af..538ebea8a 100644
--- a/src/core/renderer/render_view_observer_qt.h
+++ b/src/core/renderer/render_view_observer_qt.h
@@ -57,7 +57,6 @@ private:
     void onFetchDocumentInnerText(quint64 requestId);
     void onSetBackgroundColor(quint32 color);
 
-    void OnFirstVisuallyNonEmptyLayout() Q_DECL_OVERRIDE;
     void OnDestruct() Q_DECL_OVERRIDE { }
 
     virtual bool OnMessageReceived(const IPC::Message& message) Q_DECL_OVERRIDE;
diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp
index 0f7808e51..87badc189 100644
--- a/src/core/web_contents_delegate_qt.cpp
+++ b/src/core/web_contents_delegate_qt.cpp
@@ -51,6 +51,7 @@
 #include "file_picker_controller.h"
 #include "media_capture_devices_dispatcher.h"
 #include "network_delegate_qt.h"
+#include "render_widget_host_view_qt.h"
 #include "type_conversion.h"
 #include "web_contents_adapter_client.h"
 #include "web_contents_adapter_p.h"
@@ -387,6 +388,21 @@ void WebContentsDelegateQt::WasShown()
     web_cache::WebCacheManager::GetInstance()->ObserveActivity(web_contents()->GetRenderProcessHost()->GetID());
 }
 
+void WebContentsDelegateQt::DidFirstVisuallyNonEmptyPaint()
+{
+    RenderWidgetHostViewQt *rwhv = static_cast<RenderWidgetHostViewQt*>(web_contents()->GetRenderWidgetHostView());
+    if (!rwhv)
+        return;
+
+    RenderWidgetHostViewQt::LoadVisuallyCommittedState loadVisuallyCommittedState = rwhv->getLoadVisuallyCommittedState();
+    if (loadVisuallyCommittedState == RenderWidgetHostViewQt::NotCommitted) {
+        rwhv->setLoadVisuallyCommittedState(RenderWidgetHostViewQt::DidFirstVisuallyNonEmptyPaint);
+    } else if (loadVisuallyCommittedState == RenderWidgetHostViewQt::DidFirstCompositorFrameSwap) {
+        m_viewClient->loadVisuallyCommitted();
+        rwhv->setLoadVisuallyCommittedState(RenderWidgetHostViewQt::NotCommitted);
+    }
+}
+
 void WebContentsDelegateQt::RequestToLockMouse(content::WebContents *web_contents, bool user_gesture, bool last_unlocked_by_target)
 {
     Q_UNUSED(user_gesture);
diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h
index dad1e50f1..683b14ddf 100644
--- a/src/core/web_contents_delegate_qt.h
+++ b/src/core/web_contents_delegate_qt.h
@@ -135,6 +135,7 @@ public:
     virtual void DidUpdateFaviconURL(const std::vector<content::FaviconURL> &candidates) Q_DECL_OVERRIDE;
     virtual void DidNavigateAnyFrame(content::RenderFrameHost *render_frame_host, const content::LoadCommittedDetails &details, const content::FrameNavigateParams &params) Q_DECL_OVERRIDE;
     virtual void WasShown() Q_DECL_OVERRIDE;
+    virtual void DidFirstVisuallyNonEmptyPaint() Q_DECL_OVERRIDE;
 
     void overrideWebPreferences(content::WebContents *, content::WebPreferences*);
     void allowCertificateError(const QSharedPointer<CertificateErrorController> &) ;