New issue
Advanced search Search tips

Issue 849283 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 760181



Sign in to add a comment

Tab Capture Broken on Mac in Viz

Project Member Reported by jonr...@chromium.org, Jun 4 2018

Issue description

A bunch of tab capture tests have started failing on Mac while running in Viz.

First failing build: https://ci.chromium.org/buildbot/chromium.fyi/mac-views-rel/1947

I can reproduce on ToT today locally. From the failure it looks like 0 size requests are being sent to Viz. Here's an example failure:

[ RUN      ] SnapshotBrowserTest.AsyncMultiWindowTest

DevTools listening on ws://127.0.0.1:54574/devtools/browser/d1222580-b45a-49f2-ab7c-fe53e646f4a0
[19278:775:0604/120118.623403:ERROR:delegated_frame_host.cc(179)] Not implemented reached in void content::DelegatedFrameHost::SetNeedsBeginFrames(bool)
[19278:775:0604/120118.909542:ERROR:delegated_frame_host.cc(179)] Not implemented reached in void content::DelegatedFrameHost::SetNeedsBeginFrames(bool)
[19278:775:0604/120118.935338:ERROR:delegated_frame_host.cc(179)] Not implemented reached in void content::DelegatedFrameHost::SetNeedsBeginFrames(bool)
[19278:775:0604/120118.960401:ERROR:delegated_frame_host.cc(179)] Not implemented reached in void content::DelegatedFrameHost::SetNeedsBeginFrames(bool)
[19279:23555:0604/120118.911680:FATAL:root_compositor_frame_sink_impl.cc(74)] Check failed: !size.IsEmpty(). 
0   Content Shell Framework             0x000000010e00d01c base::debug::StackTrace::StackTrace(unsigned long) + 28
1   Content Shell Framework             0x000000010df36f6f logging::LogMessage::~LogMessage() + 223
2   Content Shell Framework             0x000000010f90e901 non-virtual thunk to viz::RootCompositorFrameSinkImpl::Resize(gfx::Size const&) + 97
3   Content Shell Framework             0x000000010bf375e3 viz::mojom::DisplayPrivateStubDispatch::Accept(viz::mojom::DisplayPrivate*, mojo::Message*) + 1747
4   Content Shell Framework             0x000000010e1328b6 mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) + 982
5   Content Shell Framework             0x000000010e132166 mojo::FilterChain::Accept(mojo::Message*) + 150
6   Content Shell Framework             0x000000010e133c95 mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) + 117
7   Content Shell Framework             0x000000010e13a32c mojo::internal::MultiplexRouter::ProcessIncomingMessage(mojo::internal::MultiplexRouter::MessageWrapper*, mojo::internal::MultiplexRouter::ClientCallBehavior, base::SequencedTaskRunner*) + 1420
8   Content Shell Framework             0x000000010e139730 mojo::internal::MultiplexRouter::Accept(mojo::Message*) + 368
9   Content Shell Framework             0x000000010e132166 mojo::FilterChain::Accept(mojo::Message*) + 150
10  Content Shell Framework             0x000000010e12d4a3 mojo::Connector::ReadSingleMessage(unsigned int*) + 419
11  Content Shell Framework             0x000000010e12df01 mojo::Connector::ReadAllAvailableMessages() + 97
12  Content Shell Framework             0x000000010e12ddb9 mojo::Connector::OnHandleReadyInternal(unsigned int) + 137
13  Content Shell Framework             0x000000010bf78c67 mojo::SimpleWatcher::DiscardReadyState(base::RepeatingCallback<void (unsigned int)> const&, unsigned int, mojo::HandleSignalsState const&) + 103
14  Content Shell Framework             0x000000010e03643d mojo::SimpleWatcher::OnHandleReady(int, unsigned int, mojo::HandleSignalsState const&) + 365
15  Content Shell Framework             0x000000010e036961 void base::internal::Invoker<base::internal::BindState<void (mojo::SimpleWatcher::*)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState>, void ()>::RunImpl<void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&, 0ul, 1ul, 2ul, 3ul>(void (mojo::SimpleWatcher::* const&&&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>) + 193
16  Content Shell Framework             0x000000010df1cbe3 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) + 323
17  Content Shell Framework             0x000000010df4ed29 base::internal::IncomingTaskQueue::RunTask(base::PendingTask*) + 121
18  Content Shell Framework             0x000000010df52bd7 base::MessageLoop::RunTask(base::PendingTask*) + 599
19  Content Shell Framework             0x000000010df52f9a base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) + 186
20  Content Shell Framework             0x000000010df5320c base::MessageLoop::DoWork() + 572
21  Content Shell Framework             0x000000010df55d68 base::MessagePumpDefault::Run(base::MessagePump::Delegate*) + 216
22  Content Shell Framework             0x000000010df525b4 base::MessageLoop::Run(bool) + 132
23  Content Shell Framework             0x000000010df871d9 base::RunLoop::Run() + 249
24  Content Shell Framework             0x000000010dfd453e base::Thread::Run(base::RunLoop*) + 206
25  Content Shell Framework             0x000000010dfd4a7e base::Thread::ThreadMain() + 782
26  Content Shell Framework             0x000000010e01c1ff base::(anonymous namespace)::ThreadFunc(void*) + 95
27  libsystem_pthread.dylib             0x00007fff7f7a26c1 _pthread_body + 340
28  libsystem_pthread.dylib             0x00007fff7f7a256d _pthread_body + 0
29  libsystem_pthread.dylib             0x00007fff7f7a1c5d thread_start + 13


samans@ any insight? Nothing in the changelist of the failure run instantly stood out to me.
 
It's definitely because of this CL. I need to understand what's happening but I'll revert first probably. https://chromium-review.googlesource.com/c/chromium/src/+/1076530
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/554a4b6a90144952665ec035d62fd60faade7412

commit 554a4b6a90144952665ec035d62fd60faade7412
Author: Saman Sami <samans@chromium.org>
Date: Mon Jun 04 23:06:45 2018

Revert "Make Compositor::DisableSwapUntilResize work with viz"

This reverts commit 3e5eaae29d310061c0504f7d011f8bb0263e0078.

Reason for revert: causes multiple bugs on Windows and breaks viz_content_browsertests on Mac FYI bots

Original change's description:
> Make Compositor::DisableSwapUntilResize work with viz
> 
> Send a sync IPC to Viz process when size is changing in order
> to prevent frame swaps of wrong size. If a wrong size frame is
> swapped then Windows might stretch our content to fill the window.
> 
> Bug:  811945 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: I4960e5a349902b75522c42a9f70b6eac531fdff6
> Reviewed-on: https://chromium-review.googlesource.com/1076530
> Reviewed-by: kylechar <kylechar@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Commit-Queue: Saman Sami <samans@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#563676}

TBR=danakj@chromium.org,rockot@chromium.org,tsepez@chromium.org,kylechar@chromium.org,samans@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  811945 ,  849283 ,  849169 ,  849155 
Change-Id: Ifada5eb26caf9fb7be5a242c0fb4b97070f4df57
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1085971
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564284}
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/content/browser/compositor/test/test_image_transport_factory.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/content/browser/compositor/viz_process_transport_factory.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/mojo/public/cpp/bindings/sync_call_restrictions.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/compositor_frame_sink_client_binding.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/compositor_frame_sink_client_binding.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/frame_generator.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/frame_generator_unittest.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/platform_display_default.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/platform_display_mirror.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/viz/privileged/interfaces/compositing/display_private.mojom
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/ui/compositor/compositor.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/ui/compositor/compositor.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/ui/compositor/test/in_process_context_factory.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/ui/compositor/test/in_process_context_factory.h

This is at least part of  issue 849478 .

Are we not to have 0x0 ui::Compositors anymore? (I can do that on macOS if so-desired, just need the heads-up).
Status: Fixed (was: Assigned)
I don't insist on not having 0x0 ui::Compositors.

Sign in to add a comment