Chromium

Note that Android currently registers/unregisters the BeginFrameSource on visibility changes. This will need to be updated as well: https://cs.chromium.org/chromium/src/content/browser/renderer_host/compositor_impl_android.cc?rcl=9f16a173c9f23cce777a78db5a278d21bf26258b&l=647

+brianderson suggested an alternative for the GPU process when I looked into BeginFrameSource + viz. We might want to use eglGetCompositorTimingANDROID() from the GPU process, although I think it's limited to newer versions of Android.

https://www.khronos.org/registry/EGL/extensions/ANDROID/EGL_ANDROID_get_frame_timestamps.txt

Registering/Unregistering BeginFrameSource on visibility doesn't make sense to me. The expensive part of BeginFrameSource is the observers because that causes a timer to be scheduled.

fsamuel: Android doesn't use a timer, it's an OS signal for each OnBeginFrame(). Also the reason it's unregistered is because the viz::Display is destroyed when visibility changes to hidden anyways. The same code to recreate viz::Display and regregister BeginFrameSource gets used in all cases.

Ohh yea, I vaguely remember this conversation. We supposedly do this to return/save resources? Do we actually know if this is still the case? Won't Display::SetVisible(false) accomplish the desired effect?

[Empty comment from Monorail migration]

Fady suggested that BeginFrames and CompositorFrameSink can be different message pipes and on Android BeginFrames can come from the browser. I think this idea would work.

Going to take a look at this.

Quick update:

It appears that the Choreographer service (which provides VSync / BeginFrame info on Android) can be run in the GPU process. There are two concerns in doing so:

#1 - Choreographer requires that the thread has a Java looper pumping it (not a native message loop). This should be a straightforward change.

#2 - There was concern that Choreographer would run on thread 0, so we'd need to move the Android GPU main thread to be thread 0 (it is currently a random thread we create later). It appears this isn't an issue. Per the docs "Each Looper thread has its own choreographer. Other threads can post callbacks to run on the choreographer but they will run on the Looper to which the choreographer belongs."

So only #1 needs to be addressed and we should be able to handle this in an ideal way. There might be some performance cost to running a Java looper, but hopefully this isn't major.

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

commit d6b04a16e2ee4091c1355debfaf5e76165a8e7c1
Author: Eric Karl <ericrk@chromium.org>
Date: Tue Jun 12 22:15:28 2018

Android OOP-D: Deliver BeginMainFrames over IPC

Currently, Android OOP-D uses a synthetic begin frame source. This
patch causes Android OOP-D to use the same Android OS driven BFS which
non-OOP-D uses, by sending the signals over an IPC to the Viz process.

A future CL will read this signal directly in the Viz process, but as
this is a riskier change, I wanted to land this first.

Bug: 811967
Change-Id: I49ace83aa0a9f27ec18f1ca3527bed4666a9ebaa
Reviewed-on: https://chromium-review.googlesource.com/1093520
Commit-Queue: Eric Karl <ericrk@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566600}
[modify] https://crrev.com/d6b04a16e2ee4091c1355debfaf5e76165a8e7c1/content/browser/BUILD.gn
[modify] https://crrev.com/d6b04a16e2ee4091c1355debfaf5e76165a8e7c1/content/browser/compositor/external_begin_frame_controller_client_impl.cc
[modify] https://crrev.com/d6b04a16e2ee4091c1355debfaf5e76165a8e7c1/content/browser/compositor/external_begin_frame_controller_client_impl.h
[modify] https://crrev.com/d6b04a16e2ee4091c1355debfaf5e76165a8e7c1/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/d6b04a16e2ee4091c1355debfaf5e76165a8e7c1/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/d6b04a16e2ee4091c1355debfaf5e76165a8e7c1/ui/compositor/compositor.h
[modify] https://crrev.com/d6b04a16e2ee4091c1355debfaf5e76165a8e7c1/ui/compositor/external_begin_frame_client.h

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

commit b2ce0204f34f426de59e14e25edc96269aacea50
Author: Eric Karl <ericrk@chromium.org>
Date: Thu Jun 21 18:36:29 2018

Android OOP-D: Use JavaHandlerThread for Display Compositor

Android OOP-D requires running a Java Choreographer based
BeginFrameSource on the display compositor thread. In order to do this,
this thread must have a Java looper.

This CL adds a looper by converting this thread from a base::Thread
to a base::android::JavaHandlerThread on Android. As we also want to
be able to specify a thread priority for this thread, this patch adds
thread priority support to the JavaHandlerThread class.

Bug: 811967
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I12d39edd5925f33629e91ad17eaf575ed49b0296
Reviewed-on: https://chromium-review.googlesource.com/1109020
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569333}
[modify] https://crrev.com/b2ce0204f34f426de59e14e25edc96269aacea50/base/android/java/src/org/chromium/base/JavaHandlerThread.java
[modify] https://crrev.com/b2ce0204f34f426de59e14e25edc96269aacea50/base/android/java_handler_thread.cc
[modify] https://crrev.com/b2ce0204f34f426de59e14e25edc96269aacea50/base/android/java_handler_thread.h
[modify] https://crrev.com/b2ce0204f34f426de59e14e25edc96269aacea50/base/test/android/java/src/org/chromium/base/JavaHandlerThreadHelpers.java
[modify] https://crrev.com/b2ce0204f34f426de59e14e25edc96269aacea50/components/viz/service/main/viz_main_impl.cc
[modify] https://crrev.com/b2ce0204f34f426de59e14e25edc96269aacea50/components/viz/service/main/viz_main_impl.h
[modify] https://crrev.com/b2ce0204f34f426de59e14e25edc96269aacea50/content/public/android/java/src/org/chromium/content/browser/LauncherThread.java

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

commit 59c094f97e16f24039d7c044d4a4ec26b14c2100
Author: Eric Karl <ericrk@chromium.org>
Date: Fri Jun 22 23:08:01 2018

Android OOP-D: ExternalBeginFrame{ControllerImpl => SourceMojo}

Make ExternalBeginFrameControllerImpl implement ExternalBeginFrameSource
directly, and rename to ExternalBeginFrameSourceMojo.

This allows GpuDisplayProvider and RootCompositorFrameSink to deal with
an ExternalBeginFrameSource directly, allowing for alternative
implementations to be dropped in in the future.

To allow this, Display assignment is moved from RootCompositorFrameSink
and a new DisplayObserver callback is added to handle cleanup/lifetime.

Bug: 811967
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I3e1475ebe6338eef4bbee5d98a06a4215c254485
Reviewed-on: https://chromium-review.googlesource.com/1109027
Commit-Queue: Eric Karl <ericrk@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569823}
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/common/frame_sinks/begin_frame_source.cc
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/common/frame_sinks/begin_frame_source.h
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/BUILD.gn
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/display/display.cc
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/display/display.h
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/display_embedder/DEPS
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/display_embedder/display_provider.h
[delete] https://crrev.com/fe3eaf210ae31aa6a657d9d879cfd24016d55c82/components/viz/service/display_embedder/external_begin_frame_controller_impl.cc
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/display_embedder/gpu_display_provider.cc
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/display_embedder/gpu_display_provider.h
[add] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/frame_sinks/external_begin_frame_source_mojo.cc
[rename] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/frame_sinks/external_begin_frame_source_mojo.h
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/test/test_display_provider.cc
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/components/viz/test/test_display_provider.h
[modify] https://crrev.com/59c094f97e16f24039d7c044d4a4ec26b14c2100/content/browser/compositor/gpu_process_transport_factory.cc

What's the status of this? Is this done or is there more work to do?

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

commit b139512f34e1fa9777475d7642d63a8e59dcba47
Author: Eric Karl <ericrk@chromium.org>
Date: Tue Jun 26 20:15:41 2018

Android OOP-D: Add ExternalBeginFrameSourceAndroid

Provides an Android Choreographer based ExternalBeginFrameSource
implementation. Allows the Viz process to produce frames in the same
manner as previously done in the Browser process.

Removes the IPC-based ExternalBeginFrameSource from CompositorImpl.

Bug: 811967
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ibf65969e2a6ddd0a9892aac99aea329ae99b94a5
Reviewed-on: https://chromium-review.googlesource.com/1109198
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570510}
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/chrome/BUILD.gn
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/chrome/android/BUILD.gn
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/components/viz/BUILD.gn
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/components/viz/service/BUILD.gn
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/components/viz/service/DEPS
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/components/viz/service/frame_sinks/DEPS
[add] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/components/viz/service/frame_sinks/external_begin_frame_source_android.cc
[add] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/components/viz/service/frame_sinks/external_begin_frame_source_android.h
[add] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/components/viz/service/frame_sinks/external_begin_frame_source_android_unittest.cc
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[add] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/components/viz/service/java/src/org/chromium/components/viz/service/frame_sinks/ExternalBeginFrameSourceAndroid.java
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/components/viz/service/main/viz_main_impl.cc
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/b139512f34e1fa9777475d7642d63a8e59dcba47/content/test/BUILD.gn

Re #13 - now it's done :D

please, review the issue,
```J.N.Mhc_M_H$ (N.java)
org.chromium.components.viz.service.frame_sinks.ExternalBeginFrameSourceAndroid.doFrame (ExternalBeginFrameSourceAndroid.java:60)
android.view.Choreographer$CallbackRecord.run (Choreographer.java:1317)
android.view.Choreographer$CallbackRecord.run (Choreographer.java:1327)
android.view.Choreographer.doCallbacks (Choreographer.java:985)
android.view.Choreographer.doFrame (Choreographer.java:911)
android.view.Choreographer$FrameDisplayEventReceiver.run (Choreographer.java:1302)
android.os.Handler.handleCallback (Handler.java:942)
android.os.Handler.dispatchMessage (Handler.java:99)
android.os.Looper.loopOnce (Looper.java:346)
android.os.Looper.loop (Looper.java:475)
android.app.ActivityThread.main (ActivityThread.java:7954)
java.lang.reflect.Method.invoke (Method.java)
com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:548)
com.android.internal.os.ZygoteInit.main (ZygoteInit.java:942)```

We also encountered a lot of ANRs about ExternalBeginFrameSourceAndroid, info is below:

Android System WebView version: 108.0.5359.128
Application: com.apkpure.aegon / com.netease.newsreader.activity

issue :
  native: #12 pc 002addfd  /data/dalvik-cache/arm/product@app@WebViewGoogle@WebViewGoogle.apk@classes.dex (art_jni_trampoline+100)
  native: #13 pc 004b6cb9  /data/dalvik-cache/arm/product@app@WebViewGoogle@WebViewGoogle.apk@classes.dex (org.chromium.components.viz.service.frame_sinks.ExternalBeginFrameSourceAndroid.doFrame+320)
  at J.N.Mhc_M_H$(Native method)
  at org.chromium.components.viz.service.frame_sinks.ExternalBeginFrameSourceAndroid.doFrame(chromium-TrichromeWebViewGoogle.aab-stable-556311633:60)
  at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1388)
  at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1398)
  at android.view.Choreographer.doCallbacks(Choreographer.java:1056)
  at android.view.Choreographer.doFrame(Choreographer.java:982)
  at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1373)
  at android.os.Handler.handleCallback(Handler.java:942)
  at android.os.Handler.dispatchMessage(Handler.java:99)
  at android.os.Looper.loopOnce(Looper.java:213)
  at android.os.Looper.loop(Looper.java:311)
  at android.app.ActivityThread.main(ActivityThread.java:7998)
  at java.lang.reflect.Method.invoke(Native method)
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:555)
  at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:957)

Please see attached file for more information

This issue was migrated from crbug.com/chromium/811967?no_tracker_redirect=1

[Monorail blocking: crbug.com/chromium/732555]
[Monorail components added to Component Tags custom field.]

Dear Google team,
The issue has been observed in latest webview version in a webview based application.

Android System WebView version: 122.0.6261.120
Application: com.towneers.www

Stack:
Cmd line: com.towneers.www
Build fingerprint: 'samsung/dm2qksx/dm2q:14/UP1A.231005.007/S916NKSS2BXAF:user/release-keys'
ABI: 'arm64'
Build type: optimized
suspend all histogram:  Sum: 290.293ms 99% C.I. 5.556us-24944.639us Avg: 759.929us Max: 42224us
DALVIK THREADS (162):
"main" prio=5 tid=1 Native
  | group="main" sCount=1 ucsCount=0 flags=1 obj=0x71c10e98 self=0xb4000079fbc72400
  | sysTid=12473 nice=-10 cgrp=top-app sched=0/0 handle=0x7ab038a500
  | state=S schedstat=( 25609942295 8088277435 67075 ) utm=1167 stm=1393 core=5 HZ=100
  | stack=0x7feb8b2000-0x7feb8b4000 stackSize=8188KB
  | held mutexes=
  native: #00 pc 000911c0  /apex/com.android.runtime/lib64/bionic/libc.so (syscall+32) (BuildId: 02a91a85343debb2911714273ff2b670)
  native: #01 pc 00095f20  /apex/com.android.runtime/lib64/bionic/libc.so (__futex_wait_ex+148) (BuildId: 02a91a85343debb2911714273ff2b670)
  native: #02 pc 000fc374  /apex/com.android.runtime/lib64/bionic/libc.so (pthread_cond_wait+80) (BuildId: 02a91a85343debb2911714273ff2b670)
  native: #03 pc 02c1c638  /data/app/~~769zoMjJDD--xZ157Lji-Q==/com.google.android.trichromelibrary_626112033-5E5Fxio546zp9q0RhIoCIA==/base.apk (offset 8dc000) (???) (BuildId: decd21b292482c13bf0c3bf9997c41544b8d11a6)
  native: #04 pc 0577ca90  /data/app/~~769zoMjJDD--xZ157Lji-Q==/com.google.android.trichromelibrary_626112033-5E5Fxio546zp9q0RhIoCIA==/base.apk (offset 8dc000) (???) (BuildId: decd21b292482c13bf0c3bf9997c41544b8d11a6)
  native: #05 pc 02c1cadc  /data/app/~~769zoMjJDD--xZ157Lji-Q==/com.google.android.trichromelibrary_626112033-5E5Fxio546zp9q0RhIoCIA==/base.apk (offset 8dc000) (???) (BuildId: decd21b292482c13bf0c3bf9997c41544b8d11a6)
  native: #06 pc 047479cc  /data/app/~~769zoMjJDD--xZ157Lji-Q==/com.google.android.trichromelibrary_626112033-5E5Fxio546zp9q0RhIoCIA==/base.apk (offset 8dc000) (???) (BuildId: decd21b292482c13bf0c3bf9997c41544b8d11a6)
  native: #07 pc 04747afc  /data/app/~~769zoMjJDD--xZ157Lji-Q==/com.google.android.trichromelibrary_626112033-5E5Fxio546zp9q0RhIoCIA==/base.apk (offset 8dc000) (???) (BuildId: decd21b292482c13bf0c3bf9997c41544b8d11a6)
  native: #08 pc 02fd7c3c  /data/app/~~769zoMjJDD--xZ157Lji-Q==/com.google.android.trichromelibrary_626112033-5E5Fxio546zp9q0RhIoCIA==/base.apk (offset 8dc000) (???) (BuildId: decd21b292482c13bf0c3bf9997c41544b8d11a6)
  native: #09 pc 044d72fc  /data/app/~~769zoMjJDD--xZ157Lji-Q==/com.google.android.trichromelibrary_626112033-5E5Fxio546zp9q0RhIoCIA==/base.apk (offset 8dc000) (Java_J_N_Mhc_1M_1H_00024+80) (BuildId: decd21b292482c13bf0c3bf9997c41544b8d11a6)
  at J.N.Mhc_M_H$(Native method)
  at org.chromium.components.viz.service.frame_sinks.ExternalBeginFrameSourceAndroid.doFrame(chromium-TrichromeWebViewGoogle6432.aab-stable-626112033:63)
  at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1648)
  at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1659)
  at android.view.Choreographer.doCallbacks(Choreographer.java:1129)
  at android.view.Choreographer.doFrame(Choreographer.java:1045)
  at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1622)
  at android.os.Handler.handleCallback(Handler.java:958)
  at android.os.Handler.dispatchMessage(Handler.java:99)
  at android.os.Looper.loopOnce(Looper.java:230)
  at android.os.Looper.loop(Looper.java:319)
  at android.app.ActivityThread.main(ActivityThread.java:8893)
  at java.lang.reflect.Method.invoke(Native method)
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:608)
  at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1103)


Kindly help in checking further.
Thanks

we also have met with the problem and it happened in different WebView versions.

[E][2024-06-25 +8.0 17:32:04.921][23717][CrashReportModuleInstaller]anr_report: pinfo_short_msg: unknown
; anr_stack: J.N.Mhc_M_H$(Native Method)
org.chromium.components.viz.service.frame_sinks.ExternalBeginFrameSourceAndroid.doFrame(chromium-TrichromeWebViewGoogle.aab-stable-647807131:63)
android.view.Choreographer$CallbackRecord.run(Choreographer.java:1054)
android.view.Choreographer.doCallbacks(Choreographer.java:880)
android.view.Choreographer.doFrame(Choreographer.java:803)
android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1041)
android.
[E][2024-06-25 +8.0 17:32:04.921][23717][CrashReportModuleInstaller]os.Handler.handleCallback(Handler.java:883)
android.os.Handler.dispatchMessage(Handler.java:100)
android.os.Looper.loop(Looper.java:227)
android.app.ActivityThread.main(ActivityThread.java:7822)
java.lang.reflect.Method.invoke(Native Method)
com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1026)

Have the same issue.

the above issue (J.N.Mhc_M_H$(Native Method)) is being tracked here
https://issues.chromium.org/issues/349274976

if you are able to consistently/frequently reproduce it, please provide info there. and vote the issue if you are impacted.
Thanks.