Closed Bug 1760794 Opened 2 years ago Closed 2 years ago

Geckoview default clear color not getting correctly set any more

Categories

(GeckoView :: General, defect)

Product:
GeckoView
Component:
General
Platform:
Unspecified
All
Type:
defect

Tracking

(firefox-esr91 unaffected, firefox98 unaffected, firefox99 fixed, firefox100 fixed)

Status:
RESOLVED FIXED
Milestone:
100 Branch
Tracking Flags:
Tracking Status
firefox-esr91 --- unaffected
firefox98 --- unaffected
firefox99 --- fixed
firefox100 --- fixed

People

(Reporter: jnicol, Assigned: jnicol)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Bug 1760794 - Ensure default clear color gets set after compositor is (re)initialized. r?agi
48 bytes, text/x-phabricator-request
dmeehan
: approval-mozilla-beta+
Details | Review

Causing white flashes again for dark mode users.

This is a regression from bug 1756700, where we delayed initialization of the compositor. This means that mUiCompositorController is null here, and the clear color therefore does not get set.

Additionally, when a compositor is recreated (which for a long time could happen when falling back to software webrender, and more recently can happen following a GPU process restart), the new compositor does not have its clear color set.

We can fix both of these issues by storing the clear color passed in to LayerViewSupport::SetDefaultClearColor(), and calling UiCompositorControllerChild::SetDefaultClearColor() from LayerViewSupport::NotifyCompositorCreated().

In bug 1756700 we delayed initializing the compositor on android. A
consequence of this is that when
LayerViewSupport::SetDefaultClearColor() gets called
mUiCompositorControllerChild is still null, meaning we skip setting
the compositor's clear color. This means that dark-mode fenix users
will start seeing white flashes while waiting for the page to load.

Additionally, when the compositor is re-initialized (following a
fallback to software rendering, or a GPU process restart) we do not
set the clear color for the new compositor, which again can lead to
white flashes.

To fix both of these issues, we cache the color value passed to
LayerViewSupport::SetDefaultClearColor(). Then whenever
LayerViewSupport::NotifyCompositorCreated() is called, we call
UiCompositorControllerChild::SetDefaultClearColor() to ensure the new
compositor uses the correct clear color.

Set release status flags based on info from the regressing bug 1756700

status-firefox100: --- → affected
status-firefox98: --- → unaffected
status-firefox99: --- → affected
status-firefox-esr91: --- → unaffected
Attachment #9268856 - Attachment description: Bug Bug 1760794 - Ensure default clear color gets set after compositor is (re)initialized. r?agi → Bug 1760794 - Ensure default clear color gets set after compositor is (re)initialized. r?agi
Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e560c3644632
Ensure default clear color gets set after compositor is (re)initialized. r=agi

Backed out for causing geckoview crashes.

Push with failures

Failure log when they fail on #displayAlreadyAcquired
Failure log when they fail on ##relaseOnDetach

Backout link

[task 2022-03-22T17:20:07.711Z] 17:20:07     INFO -  TEST-START | org.mozilla.geckoview.test.GeckoViewTest#displayAlreadyAcquired
[task 2022-03-22T17:20:08.734Z] 17:20:08     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_RESULT: shortMsg=Process crashed.
[task 2022-03-22T17:20:08.734Z] 17:20:08     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_CODE: 0
[task 2022-03-22T17:20:08.743Z] 17:20:08     INFO -  org.mozilla.geckoview.test | adb_returncode=0
[task 2022-03-22T17:20:08.746Z] 17:20:08     INFO -  Passed: 290
[task 2022-03-22T17:20:08.747Z] 17:20:08  WARNING -  Failed: 1
[task 2022-03-22T17:20:08.747Z] 17:20:08  WARNING -  One or more unittests failed.
[task 2022-03-22T17:20:08.747Z] 17:20:08     INFO -  Todo: 10
[task 2022-03-22T17:20:08.747Z] 17:20:08  WARNING -  TEST-UNEXPECTED-FAIL | runjunit.py | Some tests did not run (probably due to a crash in the harness)
[task 2022-03-22T17:20:08.747Z] 17:20:08     INFO -  SUITE-END | took 148s
[task 2022-03-22T17:20:08.886Z] 17:20:08     INFO -  mozcrash Copy/paste: /builds/worker/fetches/minidump-stackwalk/minidump-stackwalk --symbols-url=https://symbols.mozilla.org/ --human /tmp/tmp0fuqp1rn/60fa0cb6-63c0-228d-e1ef-6f01e995954d.dmp /builds/worker/workspace/build/symbols
[task 2022-03-22T17:20:11.016Z] 17:20:11     INFO -  mozcrash Saved minidump as /builds/worker/workspace/build/blobber_upload_dir/60fa0cb6-63c0-228d-e1ef-6f01e995954d.dmp
[task 2022-03-22T17:20:11.016Z] 17:20:11     INFO -  mozcrash Saved app info as /builds/worker/workspace/build/blobber_upload_dir/60fa0cb6-63c0-228d-e1ef-6f01e995954d.extra
[task 2022-03-22T17:20:11.021Z] 17:20:11  WARNING -  PROCESS-CRASH | org.mozilla.geckoview.test.GeckoViewTest#displayAlreadyAcquired | application crashed [@ mozilla::Maybe<unsigned int>::emplace<unsigned int>(unsigned int&&)]
[task 2022-03-22T17:20:11.021Z] 17:20:11     INFO -  Mozilla crash reason: MOZ_RELEASE_ASSERT(!isSome())
[task 2022-03-22T17:20:11.022Z] 17:20:11     INFO -  Crash dump filename: /tmp/tmp0fuqp1rn/60fa0cb6-63c0-228d-e1ef-6f01e995954d.dmp
[task 2022-03-22T17:20:11.022Z] 17:20:11     INFO -  Operating system: Android
[task 2022-03-22T17:20:11.022Z] 17:20:11     INFO -                    0.0.0 Linux 3.10.0+ #260 SMP PREEMPT Fri May 19 12:48:14 PDT 2017 x86_64
[task 2022-03-22T17:20:11.022Z] 17:20:11     INFO -  CPU: amd64
[task 2022-03-22T17:20:11.022Z] 17:20:11     INFO -       family 6 model 6 stepping 3
[task 2022-03-22T17:20:11.022Z] 17:20:11     INFO -       4 CPUs
[task 2022-03-22T17:20:11.022Z] 17:20:11     INFO -  Crash reason:  SIGSEGV / SEGV_MAPERR
[task 2022-03-22T17:20:11.023Z] 17:20:11     INFO -  Crash address: 0x0
[task 2022-03-22T17:20:11.023Z] 17:20:11     INFO -  Process uptime: not available
[task 2022-03-22T17:20:11.023Z] 17:20:11     INFO -  Thread 0 AndroidUI (crashed)
[task 2022-03-22T17:20:11.023Z] 17:20:11     INFO -   0  libxul.so!void mozilla::Maybe<unsigned int>::emplace<unsigned int>(unsigned int&&) [Maybe.h:e560c36446320a6bf5660f9080ea4dceb34301a9 : 844 + 0x29]
[task 2022-03-22T17:20:11.023Z] 17:20:11     INFO -      rax = 0x00007bfbd6ad3273   rdx = 0x0000000000000004
[task 2022-03-22T17:20:11.024Z] 17:20:11     INFO -      rcx = 0x00007bfbdbbc7d10   rbx = 0x00007bfbc0f13ac0
[task 2022-03-22T17:20:11.024Z] 17:20:11     INFO -      rsi = 0x00007fffdec82690   rdi = 0x000000000000001b
[task 2022-03-22T17:20:11.024Z] 17:20:11     INFO -      rbp = 0x00007fffdec837b0   rsp = 0x00007fffdec837b0
[task 2022-03-22T17:20:11.024Z] 17:20:11     INFO -       r8 = 0x000000000000ffff    r9 = 0x0000000000000000
[task 2022-03-22T17:20:11.024Z] 17:20:11     INFO -      r10 = 0x00007bfbf6aba3d0   r11 = 0x0000000000000246
[task 2022-03-22T17:20:11.025Z] 17:20:11     INFO -      r12 = 0x00007bfbf583d300   r13 = 0x00007bfbedb83560
[task 2022-03-22T17:20:11.025Z] 17:20:11     INFO -      r14 = 0x00000000ffffffff   r15 = 0x00007fffdec83874
[task 2022-03-22T17:20:11.025Z] 17:20:11     INFO -      rip = 0x00007bfbd1559517
[task 2022-03-22T17:20:11.025Z] 17:20:11     INFO -      Found by: given as instruction pointer in context
[task 2022-03-22T17:20:11.026Z] 17:20:11     INFO -   1  libxul.so!mozilla::widget::LayerViewSupport::SetDefaultClearColor(int) [nsWindow.cpp:e560c36446320a6bf5660f9080ea4dceb34301a9 : 1341 + 0xf]
[task 2022-03-22T17:20:11.026Z] 17:20:11     INFO -      rbp = 0x00007fffdec837e0   rsp = 0x00007fffdec837c0
[task 2022-03-22T17:20:11.026Z] 17:20:11     INFO -      rip = 0x00007bfbd346e83b
[task 2022-03-22T17:20:11.026Z] 17:20:11     INFO -      Found by: previous frame's frame pointer
[task 2022-03-22T17:20:11.027Z] 17:20:11     INFO -   2  libxul.so!void mozilla::jni::NativeStub<mozilla::java::GeckoSession::Compositor::SetDefaultClearColor_t, mozilla::widget::LayerViewSupport, mozilla::jni::Args<int> >::Wrap<&mozilla::widget::LayerViewSupport(int)::SetDefaultClearColor>(_JNIEnv*, _jobject*, int) [Natives.h:e560c36446320a6bf5660f9080ea4dceb34301a9 : 1447 + 0x2b]
[task 2022-03-22T17:20:11.027Z] 17:20:11     INFO -      rbp = 0x00007fffdec83840   rsp = 0x00007fffdec837f0
[task 2022-03-22T17:20:11.027Z] 17:20:11     INFO -      rip = 0x00007bfbd346b0fa
[task 2022-03-22T17:20:11.027Z] 17:20:11     INFO -      Found by: previous frame's frame pointer
[task 2022-03-22T17:20:11.028Z] 17:20:11     INFO -   3  base.odex + 0xd7400f
[task 2022-03-22T17:20:11.028Z] 17:20:11     INFO -      rbp = 0x00007fffdec838f8   rsp = 0x00007fffdec83850
[task 2022-03-22T17:20:11.028Z] 17:20:11     INFO -      rip = 0x00007bfbdd348010
[task 2022-03-22T17:20:11.028Z] 17:20:11     INFO -      Found by: previous frame's frame pointer
[task 2022-03-22T17:20:11.028Z] 17:20:11     INFO -   4  0x7fffdec83c6f
[task 2022-03-22T17:20:11.028Z] 17:20:11     INFO -      rbp = 0x00007fffdec83c70   rsp = 0x00007fffdec83908
[task 2022-03-22T17:20:11.028Z] 17:20:11     INFO -      rip = 0x00007fffdec83c70
[task 2022-03-22T17:20:11.029Z] 17:20:11     INFO -      Found by: previous frame's frame pointer
[task 2022-03-22T17:20:11.029Z] 17:20:11     INFO -   5  dalvik-LinearAlloc (deleted) + 0x355f
[task 2022-03-22T17:20:11.029Z] 17:20:11     INFO -      rbp = 0x00007fffdec83c70   rsp = 0x00007fffdec83910
[task 2022-03-22T17:20:11.029Z] 17:20:11     INFO -      rip = 0x00007bfbedb83560
[task 2022-03-22T17:20:11.029Z] 17:20:11     INFO -      Found by: stack scanning
[task 2022-03-22T17:20:11.029Z] 17:20:11     INFO -   6  base.odex + 0x92ba7e
[task 2022-03-22T17:20:11.030Z] 17:20:11     INFO -      rbp = 0x00007fffdec83c70   rsp = 0x00007fffdec83928
[task 2022-03-22T17:20:11.030Z] 17:20:11     INFO -      rip = 0x00007bfbdceffa7f
[task 2022-03-22T17:20:11.030Z] 17:20:11     INFO -      Found by: stack scanning
[task 2022-03-22T17:20:11.030Z] 17:20:11     INFO -   7  libart.so!art::ArtMethod::Invoke(art::Thread*, unsigned int*, unsigned int, art::JValue*, char const*) + 0xe7
[task 2022-03-22T17:20:11.030Z] 17:20:11     INFO -      rbp = 0x00007fffdec83c70   rsp = 0x00007fffdec83940
[task 2022-03-22T17:20:11.030Z] 17:20:11     INFO -      rip = 0x00007bfbf5251448
[task 2022-03-22T17:20:11.031Z] 17:20:11     INFO -      Found by: stack scanning
[task 2022-03-22T17:20:11.031Z] 17:20:11     INFO -   8  base.odex + 0x576cbf
[task 2022-03-22T17:20:11.031Z] 17:20:11     INFO -      rbx = 0x00007fffdec83c70   rbp = 0x0000000000000001
[task 2022-03-22T17:20:11.031Z] 17:20:11     INFO -      rsp = 0x00007fffdec83c80   r12 = 0x00007fffdec83d80
[task 2022-03-22T17:20:11.031Z] 17:20:11     INFO -      r13 = 0x00007bfbf53ed05a   r14 = 0x246a3df973aeab0b
[task 2022-03-22T17:20:11.031Z] 17:20:11     INFO -      r15 = 0x00007bfbedfdc6f8   rip = 0x00007bfbdcb4acc0
[task 2022-03-22T17:20:11.031Z] 17:20:11     INFO -      Found by: call frame info
[task 2022-03-22T17:20:11.031Z] 17:20:11     INFO -   9  base.odex + 0x563283
[task 2022-03-22T17:20:11.032Z] 17:20:11     INFO -      rsp = 0x00007fffdec83ca8   rip = 0x00007bfbdcb37284
[task 2022-03-22T17:20:11.032Z] 17:20:11     INFO -      Found by: stack scanning
[task 2022-03-22T17:20:11.032Z] 17:20:11     INFO -  10  libart.so!art::interpreter::ArtInterpreterToInterpreterBridge(art::Thread*, art::DexFile::CodeItem const*, art::ShadowFrame*, art::JValue*) + 0xac
[task 2022-03-22T17:20:11.032Z] 17:20:11     INFO -      rsp = 0x00007fffdec83cc0   rip = 0x00007bfbf53f334d
[task 2022-03-22T17:20:11.032Z] 17:20:11     INFO -      Found by: stack scanning
[task 2022-03-22T17:20:11.032Z] 17:20:11     INFO -  11  boot.oat + 0x4320b7
[task 2022-03-22T17:20:11.033Z] 17:20:11     INFO -      rsp = 0x00007fffdec83ce8   rip = 0x00000000715270b8
[task 2022-03-22T17:20:11.033Z] 17:20:11     INFO -      Found by: stack scanning
[task 2022-03-22T17:20:11.033Z] 17:20:11     INFO -  12  libart.so!art::OatDexFile::GetOatClass(unsigned short) const + 0x171
[task 2022-03-22T17:20:11.033Z] 17:20:11     INFO -      rsp = 0x00007fffdec83cf0   rip = 0x00007bfbf55575b2
[task 2022-03-22T17:20:11.033Z] 17:20:11     INFO -      Found by: stack scanning
Flags: needinfo?(jnicol)

Oh, whoops. I always forget that Maybe::emplace can only be called once.

Flags: needinfo?(jnicol)
Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cbb660716fbb
Ensure default clear color gets set after compositor is (re)initialized. r=agi

https://hg.mozilla.org/mozilla-central/rev/cbb660716fbb

Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox100: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch

Comment on attachment 9268856 [details]
Bug 1760794 - Ensure default clear color gets set after compositor is (re)initialized. r?agi

Beta/Release Uplift Approval Request

  • User impact if declined: Dark mode Fenix users will see white flashes while pages are loading
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Very minor change
  • String changes made/needed:
Attachment #9268856 - Attachment description: Bug 1760794 - Ensure default clear color gets set after compositor is (re)initialized. r?agi → Bug Bug 1760794 - Ensure default clear color gets set after compositor is (re)initialized. r?agi
Attachment #9268856 - Flags: approval-mozilla-beta?
Has Regression Range: --- → yes

Comment on attachment 9268856 [details]
Bug 1760794 - Ensure default clear color gets set after compositor is (re)initialized. r?agi

Approved for 99.0b8. Thanks.

Attachment #9268856 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9268856 - Attachment description: Bug Bug 1760794 - Ensure default clear color gets set after compositor is (re)initialized. r?agi → Bug 1760794 - Ensure default clear color gets set after compositor is (re)initialized. r?agi
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: