Chromium Code Reviews Chromium Code Reviews
Issue
1803793004:
Revert of Remove synthetic vsync from Android VsyncMonitor (Closed)
DescriptionRevert of Remove synthetic vsync from Android VsyncMonitor (patchset #2 id:20001 of https://codereview.chromium.org/1789803003/ )
Reason for revert:
This CL broke the compile on downstream bots. See https://uberchromegw.corp.google.com/i/internal.client.clank_tot/builders/clang-clankium-tot-builder/builds/22473
The relevant part of the compile log:
FindBugs run via:
java -classpath /b/build/slave/clang-clankium-tot-builder/build/src/third_party/findbugs/lib/findbugs.jar: -Xmx768m -Dfindbugs.home="/b/build/slave/clang-clankium-tot-builder/build/src/third_party/findbugs" -jar /b/build/slave/clang-clankium-tot-builder/build/src/third_party/findbugs/lib/findbugs.jar -textui -sortByClass -pluginList /b/build/slave/clang-clankium-tot-builder/build/src/tools/android/findbugs_plugin/lib/chromiumPlugin.jar -xml:withMessages -auxclasspath /b/build/slave/clang-clankium-tot-builder/build/src/third_party/android_tools/sdk/platforms/android-23/android.jar:/b/build/slave/clang-clankium-tot-builder/build/src/third_party/android_tools/sdk/extras/android/support/multidex/library/libs/android-support-multidex.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/jsr_305_javalib.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/base_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/reporter_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/base_java_test_support.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/mojo_public_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/mojo_bindings_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/device_battery_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/device_bluetooth_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/device_usb_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/device_vibration_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/media_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/mojo_system_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/net_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/ui_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/third_party/android_tools/sdk/extras/android/support/v13/android-support-v13.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/blink_headers_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/capture_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/ui_accessibility_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/midi_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/protobuf_nano_javalib.jar:/b/build/slave/clang-clankium-tot-builder/build/src/third_party/cardboard-java/src/CardboardSample/libs/cardboard.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/content_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/content_java_test_support.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/content_shell_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/base_java_unittest_support.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/chromium_apk_content_shell_apk.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/base_javatests.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/device_battery_javatests.jar:/b/build/slave/clang-clankium-tot-builder/build/src/third_party/android_tools/sdk/platforms/android-23/optional/org.apache.http.legacy.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/net_java_test_support.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/net_javatests.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/broker_java.jar -exclude /b/build/slave/clang-clankium-tot-builder/build/src/build/android/findbugs_filter/findbugs_exclude.xml /b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/chromium_apk_content_shell_test_apk.jar
FindBugs reported the following issues:
URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD: Unread public/protected field
In class org.chromium.content.browser.VSyncMonitorTest$VSyncDataCollector
Field org.chromium.content.browser.VSyncMonitorTest$VSyncDataCollector.mLastVSyncCpuTimeMillis
At VSyncMonitorTest.java:[line 47]
Usually I would just land a fix because the fix is trivial. However, I am away from my desktop
Original issue's description:
> Remove synthetic vsync from Android VsyncMonitor
>
> This logic is from the ancient past and apparently is not needed anymore.
>
> This vsync can cause BeginFrames with frame times in the past to occur.
> As a part of testing https://codereview.chromium.org/1765723002, this
> caused AwContentsGarbageCollectionTest#testCreateAndGcManyTimes to crash
> because of this. It would enter a situation where a synthetic vsync
> would be sent that tried to predict the last vsync time, but the next
> time the choreographer sent a frame time it would be *before* the
> synthetic frame time. This causes assertions later on in the pipeline
> where it's assumed that all begin frames are monotonically increasing in
> frame time.
>
> These invalid begin frames used to be dropped silently by the begin
> frame source multiplexer, but that logic is removed in the above patch.
>
> R=skyostil@chromium.org,sievers@chromium.org
>
> Committed: https://crrev.com/4a7c4f1f0ea5708d7a166a103d85d162ef3ef4c7
> Cr-Commit-Position: refs/heads/master@{#381105}
TBR=sievers@chromium.org,skyostil@chromium.org,tedchoc@chromium.org,enne@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Committed: https://crrev.com/108b1cfc07a9138a385ca0e0cca36d1e4714ff51
Cr-Commit-Position: refs/heads/master@{#381142}
Patch Set 1 #
Messages
Total messages: 6 (2 generated)
|
||||||||||||||||||||||||||||
