Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Crash type 3 in 0.23.79 #15025

Closed
bsclifton opened this issue Aug 14, 2018 · 5 comments
Closed

Crash type 3 in 0.23.79 #15025

bsclifton opened this issue Aug 14, 2018 · 5 comments

Comments

@bsclifton
Copy link
Member

bsclifton commented Aug 14, 2018

Test plan

Checks that can be done BEFORE releasing

  1. On Windows, launch Brave and open up 10-20 tabs (brave.com, basicattentiontoken.org, google.com, duckduckgo.com, etc)
  2. Minimize, maximize, and restore the window at random intervals (while browsing and switching between tabs)
  3. Make the window not full screen and resize it. Switch tabs and resize it again. Try this a fair amount
  4. Verify that no tabs crash, the browser doesn't crash, and that no reports are added to chrome://crashes

Checks that can be done AFTER releasing

  1. Login to stats.brave.com
  2. Go to Top Crash Reasons and look for 0.23.105 with the error viz::ClientLayerTreeFrameSink::SubmitCompositorFrame(viz::CompositorFrame)
  3. You shouldn't see any of these errors
0  Brave.exe!viz::ClientLayerTreeFrameSink::SubmitCompositorFrame(viz::CompositorFrame) [client_layer_tree_frame_sink.cc : 132 + 0x0]
    rax = 0x000001fc027fc8e0   rdx = 0x00000084568feae0
    rcx = 0x000000000000039d   rbx = 0x0000000000000014
    rsi = 0x000001fc0062ea70   rdi = 0x000001fc0062ea70
    rbp = 0x00000084568fe720   rsp = 0x00000084568fe6a0
     r8 = 0x0000000000000001    r9 = 0x0000000000000010
    r10 = 0x000001fc022d5850   r11 = 0x0000000000000001
    r12 = 0x000001fc021ecfa0   r13 = 0x00000084568fef90
    r14 = 0x00000084568feae0   r15 = 0x00000084568ff100
    rip = 0x00007ff7a872f54f
    Found by: given as instruction pointer in context
 1  ntdll.dll + 0x12b55
 2  Brave.exe!base::internal::flat_tree<std::pair<ui::LatencyComponentType,long long>,std::pair<std::pair<ui::LatencyComponentType,long long>,ui::LatencyInfo::LatencyComponent>,base::internal::GetKeyFromValuePairFirst<std::pair<ui::LatencyComponentType,long long>,ui::LatencyInfo::LatencyComponent>,std::less<void> >::find<std::pair<ui::LatencyComponentType,long long> > [flat_tree.h : 854 + 0x25]
 3  ntdll.dll + 0x10428
 4  Brave.exe!base::win::RegKey::RegDelRecurse(HKEY__ *,std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > const &,unsigned long) [registry.cc : 484 + 0x54]
 5  Brave.exe!viz::TransferableResource::TransferableResource(viz::TransferableResource const &) [transferable_resource.cc : 13 + 0xd]
 6  ntdll.dll + 0x10428
 7  Brave.exe!std::vector<viz::TransferableResource,std::allocator<viz::TransferableResource> >::emplace_back<const viz::TransferableResource &> [vector : 939 + 0xb]
 8  Brave.exe!std::vector<viz::TransferableResource,std::allocator<viz::TransferableResource> >::_Change_array(viz::TransferableResource * const,unsigned __int64,unsigned __int64) [vector : 1991 + 0x6b]
 9  Brave.exe!std::vector<base::HistogramBase *,std::allocator<base::HistogramBase *> >::_Reallocate_exactly [vector : 1552 + 0x12]
10  Brave.exe!base::debug::ScopedLockAcquireActivity::ScopedLockAcquireActivity(void const *,base::internal::LockImpl const *) [activity_tracker.cc : 1793 + 0xbc]
11  Brave.exe!RtlUnwindEx + 0xff0b60
12  Brave.exe!base::SampleVectorBase::Accumulate(int,int) [sample_vector.cc : 66 + 0xd]
13  Brave.exe!operator new(unsigned __int64) [new_scalar.cpp : 34 + 0x8]
14  Brave.exe!viz::CompositorFrameMetadata::CompositorFrameMetadata(viz::CompositorFrameMetadata &&) [compositor_frame_metadata.cc : 12 + 0x44]
15  Brave.exe!base::HistogramBase::FindAndRunCallback(int) [histogram_base.cc : 173 + 0x17]
16  Brave.exe!RtlUnwindEx + 0x1c6a67
17  Brave.exe!cc::LayerTreeHostImpl::DrawLayers(cc::LayerTreeHostImpl::FrameData *) [layer_tree_host_impl.cc : 2083 + 0x27]
18  Brave.exe!cc::PictureLayerImpl::AppendQuads(viz::RenderPass *,cc::AppendQuadsData *) [picture_layer_impl.cc : 579 + 0x15]

Full crash dump available here:
dump3.txt

bsclifton added a commit to brave/muon that referenced this issue Aug 21, 2018
Removing these patches from browser_plugin.cc:
- bbf2817
- 416dc9c

Most of the above patches have already been removed (after single webview) from browser_plugin.cc
Chromium 68 has a few patches related to surface synchronization, such as this:
https://chromium.googlesource.com/chromium/src/+/98d76dade64f938d939b7930e1de5460b564c2e2%5E%21/

The above Muon patches were to solve brave/browser-laptop#13982
I compiled this change in release mode + ran with 25 tabs (switching tabs with mouse/keyboard). I quit, relaunched, etc. Also turned off tab preview + used debug menu to manually discard tabs and switch back to them. Didn't notice any of the webviews get locked
Not sure if this solves the problem, but this patch might not be needed anymore

Auditors:
@bridiver, @petemill
@bsclifton
Copy link
Member Author

bsclifton commented Aug 21, 2018

Here's the block of code that's failing (line that crashes bolded):

void ClientLayerTreeFrameSink::SubmitCompositorFrame(CompositorFrame frame) {
  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
  DCHECK(compositor_frame_sink_ptr_);
  DCHECK(frame.metadata.begin_frame_ack.has_damage);
  DCHECK_LE(BeginFrameArgs::kStartingFrameNumber,
            frame.metadata.begin_frame_ack.sequence_number);

  if (!enable_surface_synchronization_) {
    local_surface_id_ =
        local_surface_id_provider_->GetLocalSurfaceIdForFrame(frame);
  } else {
    if (local_surface_id_ == last_submitted_local_surface_id_) {
      CHECK_EQ(last_submitted_device_scale_factor_,
               frame.device_scale_factor());
      CHECK_EQ(last_submitted_size_in_pixels_.height(),
               frame.size_in_pixels().height());
      CHECK_EQ(last_submitted_size_in_pixels_.width(),
               frame.size_in_pixels().width());
    }
  }

  // ...

From my investigation, it seems possible that surface sync might not be working as expected and this check is what fails. In our existing Muon codebase, we have a patch in place to disable enable_surface_synchronization_ and I was able to revert without running into any issues (tested >20 tabs for about 10-15 minutes, wasn't able to reproduce original claim)

Patch is viewable at brave/muon@0df5c9b

@bsclifton bsclifton self-assigned this Aug 21, 2018
bsclifton added a commit to brave/muon that referenced this issue Aug 28, 2018
I tested this for ~30 minutes doing resizing / minimizing / maximizing
content (including video playing) and did not notice any video tearing
or stetching

Auditors: @bridiver
bsclifton added a commit to brave/muon that referenced this issue Aug 28, 2018
Should fix brave/browser-laptop#15025

I tested this for ~30 minutes doing resizing / minimizing / maximizing
content (including video playing) and did not notice any video tearing
or stetching

Auditors: @bridiver
bsclifton added a commit to brave/muon that referenced this issue Aug 29, 2018
Should fix brave/browser-laptop#15025

I tested this for ~30 minutes doing resizing / minimizing / maximizing
content (including video playing) and did not notice any video tearing
or stetching

Auditors: @bridiver, @darkdh
@darkdh
Copy link
Member

darkdh commented Aug 29, 2018

fixed in muon 8.0.9

@darkdh darkdh closed this as completed Aug 29, 2018
@kjozwiak
Copy link
Member

@bsclifton is there anything QA can do here? I don't think we ever received/found a reproducible case that we can use. Is there any area of the browser that we should keep a close eye on when going through the the passes?

@srirambv
Copy link
Collaborator

srirambv commented Aug 29, 2018

Verification Passed on Windows 10 x64 (@srirambv):

  • 0.23.105 9a46f8f
  • Muon 8.0.9
  • libchromiumcontent 68.0.3440.84
  • Verified steps mentioned in original issue and none of the tabs crashed or browser crashed

Also went through the above STR on Win 10 x64 and didn't run into any browser/tab crashes. Used the following (@kjozwiak)

  • 0.23.105 9a46f8f
  • Muon:8.0.9
  • libchromiumcontent: 68.0.3440.84

Passed with macOS 10.12.6 using the following build:

  • 0.23.105 9a46f8f
  • Muon 8.0.9
  • libchromiumcontent 68.0.3440.84
  • Verified STR, no crashes listed in chrome://crashes

Passed on Ubuntu 17.10 x64 using the following build:

  • 0.23.105 9a46f8f
  • Muon 8.0.9
  • libchromiumcontent 68.0.3440.84

Passed on macOS 10.12.6 using the following build (@kjozwiak):

  • 0.23.105 9a46f8f
  • Muon: 8.0.9
  • libchromiumcontent: 68.0.3440.84

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.