From fbaa01eb4943f209da63df048a6de21fb84a00a8 Mon Sep 17 00:00:00 2001 From: Kelly Rossmoyer Date: Tue, 21 Aug 2018 18:06:38 -0700 Subject: Add batterystats -c data to telephony bugreports Telephony bugreports currently capture 'dumpsys batterystats' output, but only in its human-friendly format. While humans may enjoy that format, Historian does not, and fails to properly load telephony bugreports (including those triggered by LowPowerMonitor) as a result. This makes it essentially impossible to use Historian when triaging/ analyzing telephony-style bugreports. To enable Historian processing of telephony bugreports, this change adds the 'checkin' format ("dumpsys batterystats -c") data along with the existing entries. ABT (Android Bug Tool) still doesn't properly list dumpsys sections in its UI even with this change, but I think nothing short of a full, comprehensive dumpsys run will make ABT happy. And that's okay, because all of the text content can still be navigated manually. Bug: 111763716 Test: Triggered telephony bugreports with and without the change on an appropriate device, manually verified that the checkin format data was present with the code change in place, and loaded the bugreport into Historian successfully. Change-Id: Iba434bfd219c627cd3c058e549a627947d9ce501 --- cmds/dumpstate/dumpstate.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 9d897f523d..1d951bed23 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1536,6 +1536,12 @@ static void DumpstateTelephonyOnly() { RunDumpsys("APP SERVICES NON-PLATFORM", {"activity", "service", "all-non-platform"}, DUMPSYS_COMPONENTS_OPTIONS); + printf("========================================================\n"); + printf("== Checkins\n"); + printf("========================================================\n"); + + RunDumpsys("CHECKIN BATTERYSTATS", {"batterystats", "-c"}); + printf("========================================================\n"); printf("== dumpstate: done (id %d)\n", ds.id_); printf("========================================================\n"); -- cgit v1.2.3-59-g8ed1b From 738966bf16e72ca15d5cd8fcc8a6ba1533082020 Mon Sep 17 00:00:00 2001 From: Yichi Chen Date: Sat, 15 Sep 2018 14:51:18 +0800 Subject: SF: Enforce a size limitation on SurfaceTracing SurfaceTracing records operations of each frame update. It can exhaust more than 2GB on memory heap in 5 mins if display keeps updating. Enforce a size limitation on it and stop automatically to prevent memory and storage from exhausting. Note: Merge changes Iadbc1894, I3a3e499a Bug: 115434782 Test: Trigger SurfaceTracing on/off repeatedly for thread-safe Test: Enable SurfeaceTracing and let it disabled automatically Change-Id: I52d945f86a7bc501590b7c311f63a6273b9192fd Merged-In: I52d945f86a7bc501590b7c311f63a6273b9192fd --- services/surfaceflinger/SurfaceFlinger.cpp | 10 +++++-- services/surfaceflinger/SurfaceTracing.cpp | 46 ++++++++++++++++++++++-------- services/surfaceflinger/SurfaceTracing.h | 9 ++++-- 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 28b447f7aa..11e7ff0f68 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4417,6 +4417,12 @@ void SurfaceFlinger::dumpAllLocked(const Vector& args, size_t& index, mEventThread->dump(result); result.append("\n"); + /* + * Tracing state + */ + mTracing.dump(result); + result.append("\n"); + /* * HWC layer minidump */ @@ -4764,12 +4770,12 @@ status_t SurfaceFlinger::onTransact( case 1025: { // Set layer tracing n = data.readInt32(); if (n) { - ALOGV("LayerTracing enabled"); + ALOGD("LayerTracing enabled"); mTracing.enable(); doTracing("tracing.enable"); reply->writeInt32(NO_ERROR); } else { - ALOGV("LayerTracing disabled"); + ALOGD("LayerTracing disabled"); status_t err = mTracing.disable(); reply->writeInt32(err); } diff --git a/services/surfaceflinger/SurfaceTracing.cpp b/services/surfaceflinger/SurfaceTracing.cpp index f8c466ea19..67dcd06187 100644 --- a/services/surfaceflinger/SurfaceTracing.cpp +++ b/services/surfaceflinger/SurfaceTracing.cpp @@ -27,52 +27,67 @@ namespace android { void SurfaceTracing::enable() { + ATRACE_CALL(); + std::lock_guard protoGuard(mTraceMutex); + if (mEnabled) { return; } - ATRACE_CALL(); mEnabled = true; - std::lock_guard protoGuard(mTraceMutex); - mTrace.set_magic_number(uint64_t(LayersTraceFileProto_MagicNumber_MAGIC_NUMBER_H) << 32 | - LayersTraceFileProto_MagicNumber_MAGIC_NUMBER_L); + mTrace = std::make_unique(); + mTrace->set_magic_number(uint64_t(LayersTraceFileProto_MagicNumber_MAGIC_NUMBER_H) << 32 | + LayersTraceFileProto_MagicNumber_MAGIC_NUMBER_L); } status_t SurfaceTracing::disable() { + ATRACE_CALL(); + std::lock_guard protoGuard(mTraceMutex); + if (!mEnabled) { return NO_ERROR; } - ATRACE_CALL(); - std::lock_guard protoGuard(mTraceMutex); mEnabled = false; status_t err(writeProtoFileLocked()); ALOGE_IF(err == PERMISSION_DENIED, "Could not save the proto file! Permission denied"); ALOGE_IF(err == NOT_ENOUGH_DATA, "Could not save the proto file! There are missing fields"); - mTrace.Clear(); + mTrace.reset(); return err; } -bool SurfaceTracing::isEnabled() { +bool SurfaceTracing::isEnabled() const { + std::lock_guard protoGuard(mTraceMutex); return mEnabled; } void SurfaceTracing::traceLayers(const char* where, LayersProto layers) { std::lock_guard protoGuard(mTraceMutex); - - LayersTraceProto* entry = mTrace.add_entry(); + if (!mEnabled) { + return; + } + LayersTraceProto* entry = mTrace->add_entry(); entry->set_elapsed_realtime_nanos(elapsedRealtimeNano()); entry->set_where(where); entry->mutable_layers()->Swap(&layers); + + constexpr int maxBufferedEntryCount = 3600; + if (mTrace->entry_size() >= maxBufferedEntryCount) { + // TODO: flush buffered entries without disabling tracing + ALOGE("too many buffered frames; force disable tracing"); + mEnabled = false; + writeProtoFileLocked(); + mTrace.reset(); + } } status_t SurfaceTracing::writeProtoFileLocked() { ATRACE_CALL(); - if (!mTrace.IsInitialized()) { + if (!mTrace->IsInitialized()) { return NOT_ENOUGH_DATA; } std::string output; - if (!mTrace.SerializeToString(&output)) { + if (!mTrace->SerializeToString(&output)) { return PERMISSION_DENIED; } if (!android::base::WriteStringToFile(output, mOutputFileName, true)) { @@ -82,4 +97,11 @@ status_t SurfaceTracing::writeProtoFileLocked() { return NO_ERROR; } +void SurfaceTracing::dump(String8& result) const { + std::lock_guard protoGuard(mTraceMutex); + + result.appendFormat("Tracing state: %s\n", mEnabled ? "enabled" : "disabled"); + result.appendFormat(" number of entries: %d\n", mTrace ? mTrace->entry_size() : 0); +} + } // namespace android diff --git a/services/surfaceflinger/SurfaceTracing.h b/services/surfaceflinger/SurfaceTracing.h index 590ab9680f..fd8cb82a9b 100644 --- a/services/surfaceflinger/SurfaceTracing.h +++ b/services/surfaceflinger/SurfaceTracing.h @@ -18,7 +18,9 @@ #include #include +#include +#include #include using namespace android::surfaceflinger; @@ -32,9 +34,10 @@ class SurfaceTracing { public: void enable(); status_t disable(); - bool isEnabled(); + bool isEnabled() const; void traceLayers(const char* where, LayersProto); + void dump(String8& result) const; private: static constexpr auto DEFAULT_FILENAME = "/data/misc/wmtrace/layers_trace.pb"; @@ -43,8 +46,8 @@ private: bool mEnabled = false; std::string mOutputFileName = DEFAULT_FILENAME; - std::mutex mTraceMutex; - LayersTraceFileProto mTrace; + mutable std::mutex mTraceMutex; + std::unique_ptr mTrace; }; } // namespace android -- cgit v1.2.3-59-g8ed1b From bf8d7210c4bbbdc875e9695a301cdf9c3b544279 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Tue, 9 Oct 2018 15:22:46 -0700 Subject: libui: add boundary check to GraphicBuffer::unflatten Commit cb496acbe593326e8d5d563847067d02b2df40ec removed the boundary check by accident. Bug: 114223584 Test: manual Change-Id: I057bc02d5807e438530d1a5327c2e02b9d154151 --- libs/ui/GraphicBuffer.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libs/ui/GraphicBuffer.cpp b/libs/ui/GraphicBuffer.cpp index c8805000a4..6235bd6cc4 100644 --- a/libs/ui/GraphicBuffer.cpp +++ b/libs/ui/GraphicBuffer.cpp @@ -372,6 +372,10 @@ status_t GraphicBuffer::flatten(void*& buffer, size_t& size, int*& fds, size_t& status_t GraphicBuffer::unflatten( void const*& buffer, size_t& size, int const*& fds, size_t& count) { + if (size < 12 * sizeof(int)) { + android_errorWriteLog(0x534e4554, "114223584"); + return NO_MEMORY; + } int const* buf = static_cast(buffer); -- cgit v1.2.3-59-g8ed1b From 60454ab1ccaaa5c508c8a88a2426affb5c21b603 Mon Sep 17 00:00:00 2001 From: Guang Zhu Date: Tue, 17 Jul 2018 17:38:58 -0700 Subject: exclude broken test from presubmit Bug: 111454530 Bug: 118754178 Test: runng this change via forrest Change-Id: I7c1b2ae81feba7d9798cf7dcb999499b5a2cb27f Merged-In: I7c1b2ae81feba7d9798cf7dcb999499b5a2cb27f (cherry picked from commit 2e702c31452b27b757632544d0c479beee81b21b) --- services/surfaceflinger/tests/SurfaceFlinger_test.filter | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/surfaceflinger/tests/SurfaceFlinger_test.filter b/services/surfaceflinger/tests/SurfaceFlinger_test.filter index 36424b9c5e..cca84e552f 100644 --- a/services/surfaceflinger/tests/SurfaceFlinger_test.filter +++ b/services/surfaceflinger/tests/SurfaceFlinger_test.filter @@ -1,5 +1,5 @@ { "presubmit": { - "filter": "LayerTransactionTest.*:LayerUpdateTest.*:ChildLayerTest.*:SurfaceFlingerStress.*:CropLatchingTest.*:GeometryLatchingTest.*:ScreenCaptureTest.*:DereferenceSurfaceControlTest.*" + "filter": "LayerTransactionTest.*:LayerUpdateTest.*:ChildLayerTest.*:SurfaceFlingerStress.*:CropLatchingTest.*:GeometryLatchingTest.*:ScreenCaptureTest.*:DereferenceSurfaceControlTest.*:-CropLatchingTest.FinalCropLatchingBufferOldSize" } } -- cgit v1.2.3-59-g8ed1b From 256bb66d02e3cae7a439594a16228d6f462ff9a1 Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Fri, 2 Nov 2018 14:18:40 -0700 Subject: SF: Fix DisplayTransactionTest SetupNewDisplayDeviceInternalTest.createHwcVirtualDisplay was failing after presubmit unit tests were enabled on pi-dev-plus-aosp. It turned out somehow the test had gotten out of sync with the implementation, and the expectations did not match for one of the cases tested. This patch simply fixes the test to use the correct expectations for what calls are made for a HWC supported virtual display when one is created. Bug: 118887786 Test: Presubmit passes Change-Id: I366e51d81ae3cb048e7a287e4f859ac92c55a20f --- services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp index 9ac5f3b73e..9b308bfcc8 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp @@ -658,7 +658,8 @@ using NonHwcVirtualDisplayCase = using SimpleHwcVirtualDisplayVariant = HwcVirtualDisplayVariant<1024, 768, Secure::TRUE>; using HwcVirtualDisplayCase = Case; + HdrNotSupportedVariant, + NoPerFrameMetadataSupportVariant>; using WideColorP3ColorimetricDisplayCase = Case, HdrNotSupportedVariant, -- cgit v1.2.3-59-g8ed1b From 4c3137a9d3fe39d09aed664c0405bb219591cc90 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Tue, 13 Nov 2018 13:33:52 -0800 Subject: Do not use raw coordinates in VelocityTracker In P, we switched VelocityTracker to use raw coordinates when calculating the velocity. While that helped solve the issues with views being moved while touched, this has broken some other assumptions in the view hierarchy. Currently during dispatch, MotionEvent coordinates are getting adjusted as events are passed to the child views. One example is a rotated ListView. When setRotation is called on a ListView, ListView continues to use getYVelocity to determine the fling speed. However, after the view is rotated, its action is in the horizontal direction. This causes flings on ListView to not work properly. Further analysing the View system api, it appears that the problem could also occur in such cases as scale, where the view gets increased in size, and MotionEvents are adjusted accordingly. This issue could have been solved in the view hierarchy by changing the assumptions and reworking the VelocityTracker usage. However, that is deemed infeasible. The switch to raw coordinates was to resolve the issue when a moving view is trying to calculate the velocity of the fling. Since that no longer is possible, we will have to use work-arounds for those use cases. One such example is the SystemUI use case, in PanelView.java. Bug: 117921784 Bug: 117475766 Test: Sample app from bug 117475766, flings in the horizontal direction work properly Change-Id: If4cd6ace0bbef52521e1bbab8d58d649b295e2b7 --- libs/input/VelocityTracker.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/input/VelocityTracker.cpp b/libs/input/VelocityTracker.cpp index c07a81245a..c70ace0fbd 100644 --- a/libs/input/VelocityTracker.cpp +++ b/libs/input/VelocityTracker.cpp @@ -325,8 +325,8 @@ void VelocityTracker::addMovement(const MotionEvent* event) { eventTime = event->getHistoricalEventTime(h); for (size_t i = 0; i < pointerCount; i++) { uint32_t index = pointerIndex[i]; - positions[index].x = event->getHistoricalRawX(i, h); - positions[index].y = event->getHistoricalRawY(i, h); + positions[index].x = event->getHistoricalX(i, h); + positions[index].y = event->getHistoricalY(i, h); } addMovement(eventTime, idBits, positions); } @@ -334,8 +334,8 @@ void VelocityTracker::addMovement(const MotionEvent* event) { eventTime = event->getEventTime(); for (size_t i = 0; i < pointerCount; i++) { uint32_t index = pointerIndex[i]; - positions[index].x = event->getRawX(i); - positions[index].y = event->getRawY(i); + positions[index].x = event->getX(i); + positions[index].y = event->getY(i); } addMovement(eventTime, idBits, positions); } -- cgit v1.2.3-59-g8ed1b From 07c3bc4ef7006656de8b7e697172a11a40cbca2c Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Fri, 2 Nov 2018 14:18:40 -0700 Subject: SF: Fix DisplayTransactionTest SetupNewDisplayDeviceInternalTest.createHwcVirtualDisplay was failing after presubmit unit tests were enabled on pi-dev-plus-aosp. It turned out somehow the test had gotten out of sync with the implementation, and the expectations did not match for one of the cases tested. This patch simply fixes the test to use the correct expectations for what calls are made for a HWC supported virtual display when one is created. Bug: 118887786 Test: Presubmit passes Change-Id: I366e51d81ae3cb048e7a287e4f859ac92c55a20f (cherry picked from commit 256bb66d02e3cae7a439594a16228d6f462ff9a1) --- services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp index 9ac5f3b73e..9b308bfcc8 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp @@ -658,7 +658,8 @@ using NonHwcVirtualDisplayCase = using SimpleHwcVirtualDisplayVariant = HwcVirtualDisplayVariant<1024, 768, Secure::TRUE>; using HwcVirtualDisplayCase = Case; + HdrNotSupportedVariant, + NoPerFrameMetadataSupportVariant>; using WideColorP3ColorimetricDisplayCase = Case, HdrNotSupportedVariant, -- cgit v1.2.3-59-g8ed1b