Improve the timing of OnLoadEnd (fixes issue #3341)

Use WebContentsDelegate::DidFinishLoad instead of a custom Mojo message.
This fixes flaky OnLoadEnd behavior with NavigationTest.Order.

libcef/browser/browser_contents_delegate.cc

@@ -515,6 +515,20 @@ void CefBrowserContentsDelegate::DidFailLoad(
   OnLoadEnd(frame, validated_url, error_code);
 }
 
+void CefBrowserContentsDelegate::DidFinishLoad(
+    content::RenderFrameHost* render_frame_host,
+    const GURL& validated_url) {
+  auto frame = browser_info_->GetFrameForHost(render_frame_host);
+  frame->RefreshAttributes();
+
+  int http_status_code = 0;
+  if (auto response_headers = render_frame_host->GetLastResponseHeaders()) {
+    http_status_code = response_headers->response_code();
+  }
+
+  OnLoadEnd(frame, validated_url, http_status_code);
+}
+
 void CefBrowserContentsDelegate::TitleWasSet(content::NavigationEntry* entry) {
   // |entry| may be NULL if a popup is created via window.open and never
   // navigated.
@@ -577,17 +591,6 @@ void CefBrowserContentsDelegate::Observe(
   }
 }
 
-void CefBrowserContentsDelegate::OnLoadEnd(CefRefPtr<CefFrame> frame,
-                                           const GURL& url,
-                                           int http_status_code) {
-  if (auto c = client()) {
-    if (auto handler = c->GetLoadHandler()) {
-      auto navigation_lock = browser_info_->CreateNavigationLock();
-      handler->OnLoadEnd(browser(), frame, http_status_code);
-    }
-  }
-}
-
 bool CefBrowserContentsDelegate::OnSetFocus(cef_focus_source_t source) {
   // SetFocus() might be called while inside the OnSetFocus() callback. If
   // so, don't re-enter the callback.
@@ -649,6 +652,17 @@ void CefBrowserContentsDelegate::OnLoadStart(
   }
 }
 
+void CefBrowserContentsDelegate::OnLoadEnd(CefRefPtr<CefFrame> frame,
+                                           const GURL& url,
+                                           int http_status_code) {
+  if (auto c = client()) {
+    if (auto handler = c->GetLoadHandler()) {
+      auto navigation_lock = browser_info_->CreateNavigationLock();
+      handler->OnLoadEnd(browser(), frame, http_status_code);
+    }
+  }
+}
+
 void CefBrowserContentsDelegate::OnLoadError(CefRefPtr<CefFrame> frame,
                                              const GURL& url,
                                              int error_code) {

libcef/browser/browser_contents_delegate.h

@@ -134,6 +134,8 @@ class CefBrowserContentsDelegate : public content::WebContentsDelegate,
   void DidFailLoad(content::RenderFrameHost* render_frame_host,
                    const GURL& validated_url,
                    int error_code) override;
+  void DidFinishLoad(content::RenderFrameHost* render_frame_host,
+                     const GURL& validated_url) override;
   void TitleWasSet(content::NavigationEntry* entry) override;
   void DidUpdateFaviconURL(
       content::RenderFrameHost* render_frame_host,
@@ -159,9 +161,6 @@ class CefBrowserContentsDelegate : public content::WebContentsDelegate,
 
   // Helpers for executing client callbacks.
   // TODO(cef): Make this private if/when possible.
-  void OnLoadEnd(CefRefPtr<CefFrame> frame,
-                 const GURL& url,
-                 int http_status_code);
   bool OnSetFocus(cef_focus_source_t source);
 
  private:
@@ -173,6 +172,9 @@ class CefBrowserContentsDelegate : public content::WebContentsDelegate,
   void OnAddressChange(const GURL& url);
   void OnLoadStart(CefRefPtr<CefFrame> frame,
                    ui::PageTransition transition_type);
+  void OnLoadEnd(CefRefPtr<CefFrame> frame,
+                 const GURL& url,
+                 int http_status_code);
   void OnLoadError(CefRefPtr<CefFrame> frame, const GURL& url, int error_code);
   void OnTitleChange(const std::u16string& title);
   void OnFullscreenModeChange(bool fullscreen);

libcef/browser/browser_frame.cc

@@ -62,13 +62,6 @@ void CefBrowserFrame::FrameAttached(
   }
 }
 
-void CefBrowserFrame::DidFinishFrameLoad(const GURL& validated_url,
-                                         int http_status_code) {
-  if (auto host = GetFrameHost()) {
-    host->DidFinishFrameLoad(validated_url, http_status_code);
-  }
-}
-
 void CefBrowserFrame::UpdateDraggableRegions(
     absl::optional<std::vector<cef::mojom::DraggableRegionEntryPtr>> regions) {
   if (auto host = GetFrameHost()) {

libcef/browser/browser_frame.h

@@ -40,8 +40,6 @@ class CefBrowserFrame
                               base::ReadOnlySharedMemoryRegion region) override;
   void FrameAttached(mojo::PendingRemote<cef::mojom::RenderFrame> render_frame,
                      bool reattached) override;
-  void DidFinishFrameLoad(const GURL& validated_url,
-                          int32_t http_status_code) override;
   void UpdateDraggableRegions(
       absl::optional<std::vector<cef::mojom::DraggableRegionEntryPtr>> regions)
       override;

libcef/browser/browser_host_base.cc

@@ -848,14 +848,6 @@ bool CefBrowserHostBase::Navigate(const content::OpenURLParams& params) {
   return false;
 }
 
-void CefBrowserHostBase::OnDidFinishLoad(CefRefPtr<CefFrameHostImpl> frame,
-                                         const GURL& validated_url,
-                                         int http_status_code) {
-  frame->RefreshAttributes();
-
-  contents_delegate_->OnLoadEnd(frame, validated_url, http_status_code);
-}
-
 void CefBrowserHostBase::ViewText(const std::string& text) {
   if (!CEF_CURRENTLY_ON_UIT()) {
     CEF_POST_TASK(CEF_UIT,

libcef/browser/browser_host_base.h

@@ -247,9 +247,6 @@ class CefBrowserHostBase : public CefBrowserHost,
 
   // Methods called from CefFrameHostImpl.
   void LoadMainFrameURL(const content::OpenURLParams& params);
-  void OnDidFinishLoad(CefRefPtr<CefFrameHostImpl> frame,
-                       const GURL& validated_url,
-                       int http_status_code);
   virtual void OnSetFocus(cef_focus_source_t source) = 0;
   void ViewText(const std::string& text);
 

libcef/browser/frame_host_impl.cc

@@ -647,13 +647,6 @@ void CefFrameHostImpl::FrameAttached(
       CefRefPtr<CefFrameHostImpl>(this), reattached));
 }
 
-void CefFrameHostImpl::DidFinishFrameLoad(const GURL& validated_url,
-                                          int http_status_code) {
-  auto browser = GetBrowserHostBase();
-  if (browser)
-    browser->OnDidFinishLoad(this, validated_url, http_status_code);
-}
-
 void CefFrameHostImpl::UpdateDraggableRegions(
     absl::optional<std::vector<cef::mojom::DraggableRegionEntryPtr>> regions) {
   auto browser = GetBrowserHostBase();

libcef/browser/frame_host_impl.h

@@ -136,8 +136,6 @@ class CefFrameHostImpl : public CefFrame, public cef::mojom::BrowserFrame {
                               base::ReadOnlySharedMemoryRegion region) override;
   void FrameAttached(mojo::PendingRemote<cef::mojom::RenderFrame> render_frame,
                      bool reattached) override;
-  void DidFinishFrameLoad(const GURL& validated_url,
-                          int32_t http_status_code) override;
   void UpdateDraggableRegions(
       absl::optional<std::vector<cef::mojom::DraggableRegionEntryPtr>> regions)
       override;

libcef/common/mojom/cef.mojom

@@ -93,9 +93,6 @@ interface BrowserFrame {
   FrameAttached(pending_remote<RenderFrame> render_frame,
                 bool reattached);
 
-  // The render frame has finished loading.
-  DidFinishFrameLoad(url.mojom.Url validated_url, int32 http_status_code);
-
   // Draggable regions have updated.
   UpdateDraggableRegions(array<DraggableRegionEntry>? regions);
 };

libcef/renderer/frame_impl.cc

@@ -364,15 +364,6 @@ void CefFrameImpl::OnDidFinishLoad() {
   blink::WebDocumentLoader* dl = frame_->GetDocumentLoader();
   const int http_status_code = dl->GetWebResponse().HttpStatusCode();
 
-  SendToBrowserFrame(__FUNCTION__,
-                     base::BindOnce(
-                         [](const GURL& url, int http_status_code,
-                            const BrowserFrameType& browser_frame) {
-                           browser_frame->DidFinishFrameLoad(url,
-                                                             http_status_code);
-                         },
-                         dl->GetUrl(), http_status_code));
-
   CefRefPtr<CefApp> app = CefAppManager::Get()->GetApplication();
   if (app) {
     CefRefPtr<CefRenderProcessHandler> handler = app->GetRenderProcessHandler();

tests/ceftests/pdf_viewer_unittest.cc

@@ -92,6 +92,9 @@ class PdfViewerTestHandler : public TestHandler, public CefContextMenuHandler {
                  int httpStatusCode) override {
     bool is_pdf1 = false;
     const std::string& url = frame->GetURL();
+    if (url == "about:blank")
+      return;
+
     if (url == kPdfHtmlUrl) {
       if (!got_on_load_end_html_)
         got_on_load_end_html_.yes();
@@ -107,7 +110,7 @@ class PdfViewerTestHandler : public TestHandler, public CefContextMenuHandler {
         NOTREACHED();
       }
     } else {
-      NOTREACHED();
+      NOTREACHED() << "url=" << url;
     }
 
     if (is_pdf1) {

tests/ceftests/resource_request_handler_unittest.cc

@@ -842,10 +842,15 @@ class BasicResponseTest : public TestHandler {
     EXPECT_EQ(browser_id_, browser->GetIdentifier());
     EXPECT_TRUE(frame->IsMain());
 
-    if (unhandled_)
+    if (unhandled_) {
-      EXPECT_EQ(httpStatusCode, 0);
+      if (IsRedirect()) {
-    else
+        EXPECT_EQ(httpStatusCode, 307);
+      } else {
+        EXPECT_EQ(httpStatusCode, 0);
+      }
+    } else {
       EXPECT_EQ(httpStatusCode, 200);
+    }
 
     on_load_end_ct_++;