OSR SendExternalBeginFrame does not call OnPaint as expected

Issue #2800 wontfix
Marshall Greenblatt created an issue

What steps will reproduce the problem?

Create a minimal OSR app that:

  1. Sets CefSettings.windowless_rendering_enabled = true.
  2. Creates a browser with CefWindowInfo.SetAsWindowless and CefWindowInfo.external_begin_frame_enabled = true.
  3. Returns a fixed size from CefRenderHandler::GetViewRect and GetScreenInfo.
  4. Loads a static HTML file with LoadURL.
  5. Calls WasResized and SendExternalBeginFrame.

What is the expected output? What do you see instead?

The expected image should be delivered to the OnPaint callback. Instead, any of the following may occur:

  1. The OnPaint callback is never executed.
  2. The OnPaint callback receives a completely black image of the correct size.

What version of the product are you using? On what operating system?

The problem reproduces with CEF v78 and v79 on Windows 10 and Linux. It can occur both with GPU enabled and disabled.

The problem does not reproduce with v73 (version that added begin-frame support prior to the Viz refactoring).

Comments (7)

  1. Marshall Greenblatt reporter

    I’m not sure how this is supposed to work. SendExternalBeginFrame results in a call to ExternalBeginFrameSourceMojo::IssueExternalBeginFrame in the GPU process which results in a call to AsyncLayerTreeFrameSink::OnBeginFrame in the renderer process. That call does nothing because cc::Scheduler doesn’t implement OnBeginFrame.

    The call to SetNeedsOneBeginFrame from ExternalBeginFrameSourceMojo::IssueExternalBeginFrame does result in a call to CefRenderWidgetHostViewOSR::OnDisplayDidFinishFrame as the code comments suggest, but that method implementation is currently empty in CEF.

  2. Marshall Greenblatt reporter

    Here’s the proposed unit test for this functionality against current master. Once this is working I would expect OnPaint to be called and the existing IsBackgroundInBuffer check to pass, but that may need further adjustment.

    diff --git tests/ceftests/os_rendering_unittest.cc tests/ceftests/os_rendering_unittest.cc
    index cae19438..794d4b33 100644
    --- tests/ceftests/os_rendering_unittest.cc
    +++ tests/ceftests/os_rendering_unittest.cc
    @@ -170,6 +170,8 @@ enum OSRTestType {
       OSR_TEST_TOUCH_CANCEL,
       // CEF_POINTER_TYPE_PEN is mapped to pen pointer event
       OSR_TEST_PEN,
    +  // Test external begin frame.
    +  OSR_TEST_EXTERNAL_BEGIN_FRAME,
       // Define the range for popup tests.
       OSR_TEST_POPUP_FIRST = OSR_TEST_POPUP_PAINT,
       OSR_TEST_POPUP_LAST = OSR_TEST_POPUP_SCROLL_INSIDE,
    @@ -208,8 +210,15 @@ class OSRTestHandler : public RoutingTestHandler,
       void OnLoadEnd(CefRefPtr<CefBrowser> browser,
                      CefRefPtr<CefFrame> frame,
                      int httpStatusCode) override {
    -    if (!started())
    +    if (!started()) {
    +      if (test_type_ == OSR_TEST_EXTERNAL_BEGIN_FRAME) {
    +        // This should trigger a call to OnPaint().
    +        browser->GetHost()->WasResized();
    +        browser->GetHost()->SendExternalBeginFrame();
    +      }
    +
           return;
    +    }
    
         switch (test_type_) {
           case OSR_TEST_KEY_EVENTS: {
    @@ -526,6 +535,7 @@ class OSRTestHandler : public RoutingTestHandler,
         // Send events after the first full repaint
         switch (test_type_) {
           case OSR_TEST_PAINT:
    +      case OSR_TEST_EXTERNAL_BEGIN_FRAME:
             if (StartTest()) {
               // test that we have a full repaint
               EXPECT_EQ(dirtyRects.size(), 1U);
    @@ -1324,6 +1334,10 @@ class OSRTestHandler : public RoutingTestHandler,
           settings.background_color = CefColorSetARGB(255, 255, 255, 255);
         }
    
    +    if (test_type_ == OSR_TEST_EXTERNAL_BEGIN_FRAME) {
    +      windowInfo.external_begin_frame_enabled = true;
    +    }
    +
     #if defined(OS_WIN)
         windowInfo.SetAsWindowless(GetDesktopWindow());
     #elif defined(OS_MACOSX)
    @@ -1603,3 +1617,5 @@ OSR_TEST(TouchEnd2X, OSR_TEST_TOUCH_END, 2.0f)
     OSR_TEST(TouchCancel, OSR_TEST_TOUCH_CANCEL, 1.0f)
     OSR_TEST(TouchCancel2X, OSR_TEST_TOUCH_CANCEL, 2.0f)
     OSR_TEST(PenEvent, OSR_TEST_PEN, 1.0f)
    +OSR_TEST(ExternalBeginFrame, OSR_TEST_EXTERNAL_BEGIN_FRAME, 1.0f)
    +OSR_TEST(ExternalBeginFrame2X, OSR_TEST_EXTERNAL_BEGIN_FRAME, 2.0f)
    

  3. Marshall Greenblatt reporter

    Testing with cefclient.exe --off-screen-rendering-enabled --external-begin-frame-enabled there appear to be many SendExternalBeginFrame() calls before the first resulting call to OnPaint(). The callback to CefLayeredWindowUpdaterOSR::Draw arrives via this callstack in the GPU process:

    >   service.dll!viz::mojom::LayeredWindowUpdaterProxy::Draw(const gfx::Rect & in_damage_rect, base::OnceCallback<void ()> callback) Line 123    C++
        service.dll!viz::SoftwareOutputDeviceProxy::EndPaint() Line 127 C++
        service.dll!viz::SoftwareRenderer::FinishDrawingFrame() Line 96 C++
        service.dll!viz::DirectRenderer::DrawFrame(std::__1::vector<std::__1::unique_ptr<viz::RenderPass,std::__1::default_delete<viz::RenderPass>>,std::__1::allocator<std::__1::unique_ptr<viz::RenderPass,std::__1::default_delete<viz::RenderPass>>>> * render_passes_in_draw_order, float device_scale_factor, const gfx::Size & device_viewport_size, float sdr_white_level) Line 456 C++
        service.dll!viz::Display::DrawAndSwap() Line 535    C++
        service.dll!viz::DisplayScheduler::DrawAndSwap() Line 215   C++
        service.dll!viz::DisplayScheduler::AttemptDrawAndSwap() Line 487    C++
        service.dll!viz::DisplayScheduler::OnBeginFrameDeadline() Line 503  C++
    

    The OnBeginFrameDeadline call is triggered via this callstack in the GPU process:

    >   service.dll!viz::DisplayScheduler::ScheduleBeginFrameDeadline() Line 440    C++
        service.dll!viz::DisplayScheduler::OnBeginFrameDerivedImpl(const viz::BeginFrameArgs & args) Line 266   C++
        viz_common.dll!viz::BeginFrameObserverBase::OnBeginFrame(const viz::BeginFrameArgs & args) Line 92  C++
        viz_common.dll!viz::`anonymous namespace'::FilterAndIssueBeginFrame(viz::BeginFrameObserver * observer, const viz::BeginFrameArgs & args) Line 46   C++
        viz_common.dll!viz::ExternalBeginFrameSource::OnBeginFrame(const viz::BeginFrameArgs & args) Line 464   C++
        service.dll!viz::ExternalBeginFrameSourceMojo::IssueExternalBeginFrame(const viz::BeginFrameArgs & args) Line 23    C++
    

    Which originates from the SendExternalBeginFrame call in the test:

    >   content.dll!viz::mojom::ExternalBeginFrameControllerProxy::IssueExternalBeginFrame(const viz::BeginFrameArgs & in_args) Line 59 C++
        content.dll!ui::HostContextFactoryPrivate::IssueExternalBeginFrame(ui::Compositor * compositor, const viz::BeginFrameArgs & args) Line 275  C++
        libcef.dll!CefRenderWidgetHostViewOSR::SendExternalBeginFrame() Line 939    C++
        libcef.dll!CefBrowserPlatformDelegateOsr::SendExternalBeginFrame() Line 236 C++
        libcef.dll!CefBrowserHostImpl::SendExternalBeginFrame() Line 1154   C++
    

    For the unit test DisplayScheduler::OnBeginFrameDeadline is called but not LayeredWindowUpdaterProxy::Draw.

  4. Marshall Greenblatt reporter

    Looks like the problem for the unit test is that DisplayScheduler::ShouldDraw is returning false when called fromDisplayScheduler::AttemptDrawAndSwap due to root_frame_missing_ being true. That variable is set to false a bit later via this GPU process call stack:

    >   service.dll!viz::DisplayScheduler::SetRootFrameMissing(bool missing) Line 73    C++
        service.dll!viz::Display::UpdateRootFrameMissing() Line 403 C++
        service.dll!viz::Display::SetLocalSurfaceId(const viz::LocalSurfaceId & id, float device_scale_factor) Line 240 C++
        service.dll!viz::RootCompositorFrameSinkImpl::SubmitCompositorFrame(const viz::LocalSurfaceId & local_surface_id, viz::CompositorFrame frame, base::Optional<viz::HitTestRegionList> hit_test_region_list, unsigned __int64 submit_time) Line 251   C++
    

    Which originates from this call stack in the main process:

    >   cc_mojo_embedder.dll!viz::mojom::CompositorFrameSinkProxy::SubmitCompositorFrame(const viz::LocalSurfaceId & in_local_surface_id, viz::CompositorFrame in_frame, base::Optional<viz::HitTestRegionList> in_hit_test_region_list, unsigned __int64 in_submit_time) Line 183  C++
        cc_mojo_embedder.dll!cc::mojo_embedder::AsyncLayerTreeFrameSink::SubmitCompositorFrame(viz::CompositorFrame frame, bool hit_test_data_changed, bool show_hit_test_borders) Line 252 C++
        cc.dll!cc::LayerTreeHostImpl::DrawLayers(cc::LayerTreeHostImpl::FrameData * frame) Line 2239    C++
        cc.dll!cc::SingleThreadProxy::DoComposite(cc::LayerTreeHostImpl::FrameData * frame) Line 687    C++
        cc.dll!cc::SingleThreadProxy::ScheduledActionDrawIfPossible() Line 904  C++
        cc.dll!cc::Scheduler::DrawIfPossible() Line 729 C++
        cc.dll!cc::Scheduler::ProcessScheduledActions() Line 832    C++
        cc.dll!cc::Scheduler::OnBeginImplFrameDeadline() Line 717   C++
    

  5. Marshall Greenblatt reporter

    I wonder if there’s some callback that we need to wait for (to guarantee that SubmitCompositorFrame has finished its setup) before calling SendExternalBeginFrame.

  6. Marshall Greenblatt reporter

    Marking old issues that we're unlikely to address as WontFix. If it still applies with currently supported CEF versions, and someone would like to submit a PR fix, then we can reopen.

  7. Log in to comment