diff options
| -rw-r--r-- | libs/dumputils/dump_utils.cpp | 36 | ||||
| -rw-r--r-- | libs/gui/tests/EndToEndNativeInputTest.cpp | 18 | ||||
| -rw-r--r-- | libs/sensor/Sensor.cpp | 3 | ||||
| -rw-r--r-- | libs/ui/GraphicBufferAllocator.cpp | 13 | ||||
| -rw-r--r-- | libs/ui/include/ui/GraphicBufferAllocator.h | 2 | ||||
| -rw-r--r-- | libs/ui/tests/Android.bp | 20 | ||||
| -rw-r--r-- | libs/ui/tests/GraphicBufferAllocator_test.cpp | 116 | ||||
| -rw-r--r-- | libs/ui/tests/mock/MockGrallocAllocator.cpp | 27 | ||||
| -rw-r--r-- | libs/ui/tests/mock/MockGrallocAllocator.h | 44 | ||||
| -rw-r--r-- | services/surfaceflinger/ContainerLayer.cpp | 4 | ||||
| -rw-r--r-- | services/surfaceflinger/ContainerLayer.h | 2 | ||||
| -rw-r--r-- | services/surfaceflinger/Layer.cpp | 2 | ||||
| -rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 3 |
13 files changed, 267 insertions, 23 deletions
diff --git a/libs/dumputils/dump_utils.cpp b/libs/dumputils/dump_utils.cpp index 250f902f9d..047780171e 100644 --- a/libs/dumputils/dump_utils.cpp +++ b/libs/dumputils/dump_utils.cpp @@ -16,7 +16,9 @@ #include <set> #include <android-base/file.h> +#include <android-base/properties.h> #include <android-base/stringprintf.h> +#include <android-base/strings.h> #include <android/hidl/manager/1.0/IServiceManager.h> #include <dumputils/dump_utils.h> #include <log/log.h> @@ -62,16 +64,40 @@ static const char* hal_interfaces_to_dump[] { "android.hardware.sensors@1.0::ISensors", "android.hardware.thermal@2.0::IThermal", "android.hardware.vr@1.0::IVr", + "android.hardware.automotive.audiocontrol@1.0::IAudioControl", + "android.hardware.automotive.vehicle@2.0::IVehicle", + "android.hardware.automotive.evs@1.0::IEvsCamera", NULL, }; -bool should_dump_hal_interface(const char* interface) { +/* list of extra hal interfaces to dump containing process during native dumps */ +// This is filled when dumpstate is called. +static std::set<const std::string> extra_hal_interfaces_to_dump; + +static void read_extra_hals_to_dump_from_property() { + // extra hals to dump are already filled + if (extra_hal_interfaces_to_dump.size() > 0) { + return; + } + std::string value = android::base::GetProperty("ro.dump.hals.extra", ""); + std::vector<std::string> tokens = android::base::Split(value, ","); + for (const auto &token : tokens) { + std::string trimmed_token = android::base::Trim(token); + if (trimmed_token.length() == 0) { + continue; + } + extra_hal_interfaces_to_dump.insert(trimmed_token); + } +} + +// check if interface is included in either default hal list or extra hal list +bool should_dump_hal_interface(const std::string& interface) { for (const char** i = hal_interfaces_to_dump; *i; i++) { - if (!strcmp(*i, interface)) { + if (interface == *i) { return true; } } - return false; + return extra_hal_interfaces_to_dump.find(interface) != extra_hal_interfaces_to_dump.end(); } bool should_dump_native_traces(const char* path) { @@ -91,13 +117,15 @@ std::set<int> get_interesting_hal_pids() { sp<IServiceManager> manager = IServiceManager::getService(); std::set<int> pids; + read_extra_hals_to_dump_from_property(); + Return<void> ret = manager->debugDump([&](auto& hals) { for (const auto &info : hals) { if (info.pid == static_cast<int>(IServiceManager::PidConstant::NO_PID)) { continue; } - if (!should_dump_hal_interface(info.interfaceName.c_str())) { + if (!should_dump_hal_interface(info.interfaceName)) { continue; } diff --git a/libs/gui/tests/EndToEndNativeInputTest.cpp b/libs/gui/tests/EndToEndNativeInputTest.cpp index 386f731d23..0babd23573 100644 --- a/libs/gui/tests/EndToEndNativeInputTest.cpp +++ b/libs/gui/tests/EndToEndNativeInputTest.cpp @@ -435,9 +435,11 @@ TEST_F(InputSurfacesTest, input_ignores_transparent_region) { surface->expectTap(1, 1); } -// Ensure we send the input to the right surface when the surface visibility changes due to the -// first buffer being submitted. ref: b/120839715 -TEST_F(InputSurfacesTest, input_respects_buffer_layer_buffer) { +// TODO(b/139494112) update tests once we define expected behavior +// Ensure we still send input to the surface regardless of surface visibility changes due to the +// first buffer being submitted or alpha changes. +// Original bug ref: b/120839715 +TEST_F(InputSurfacesTest, input_ignores_buffer_layer_buffer) { std::unique_ptr<InputSurface> bgSurface = makeSurface(100, 100); std::unique_ptr<InputSurface> bufferSurface = InputSurface::makeBufferInputSurface(mComposerClient, 100, 100); @@ -446,14 +448,14 @@ TEST_F(InputSurfacesTest, input_respects_buffer_layer_buffer) { bufferSurface->showAt(10, 10); injectTap(11, 11); - bgSurface->expectTap(1, 1); + bufferSurface->expectTap(1, 1); postBuffer(bufferSurface->mSurfaceControl); injectTap(11, 11); bufferSurface->expectTap(1, 1); } -TEST_F(InputSurfacesTest, input_respects_buffer_layer_alpha) { +TEST_F(InputSurfacesTest, input_ignores_buffer_layer_alpha) { std::unique_ptr<InputSurface> bgSurface = makeSurface(100, 100); std::unique_ptr<InputSurface> bufferSurface = InputSurface::makeBufferInputSurface(mComposerClient, 100, 100); @@ -468,10 +470,10 @@ TEST_F(InputSurfacesTest, input_respects_buffer_layer_alpha) { bufferSurface->doTransaction([](auto &t, auto &sc) { t.setAlpha(sc, 0.0); }); injectTap(11, 11); - bgSurface->expectTap(1, 1); + bufferSurface->expectTap(1, 1); } -TEST_F(InputSurfacesTest, input_respects_color_layer_alpha) { +TEST_F(InputSurfacesTest, input_ignores_color_layer_alpha) { std::unique_ptr<InputSurface> bgSurface = makeSurface(100, 100); std::unique_ptr<InputSurface> fgSurface = makeSurface(100, 100); @@ -484,7 +486,7 @@ TEST_F(InputSurfacesTest, input_respects_color_layer_alpha) { fgSurface->doTransaction([](auto &t, auto &sc) { t.setAlpha(sc, 0.0); }); injectTap(11, 11); - bgSurface->expectTap(1, 1); + fgSurface->expectTap(1, 1); } TEST_F(InputSurfacesTest, input_respects_container_layer_visiblity) { diff --git a/libs/sensor/Sensor.cpp b/libs/sensor/Sensor.cpp index 139987e6a9..abc910302c 100644 --- a/libs/sensor/Sensor.cpp +++ b/libs/sensor/Sensor.cpp @@ -577,7 +577,8 @@ void Sensor::flattenString8(void*& buffer, size_t& size, uint32_t len = static_cast<uint32_t>(string8.length()); FlattenableUtils::write(buffer, size, len); memcpy(static_cast<char*>(buffer), string8.string(), len); - FlattenableUtils::advance(buffer, size, FlattenableUtils::align<4>(len)); + FlattenableUtils::advance(buffer, size, len); + size -= FlattenableUtils::align<4>(buffer); } bool Sensor::unflattenString8(void const*& buffer, size_t& size, String8& outputString8) { diff --git a/libs/ui/GraphicBufferAllocator.cpp b/libs/ui/GraphicBufferAllocator.cpp index f1aa89ca36..d02bb94a05 100644 --- a/libs/ui/GraphicBufferAllocator.cpp +++ b/libs/ui/GraphicBufferAllocator.cpp @@ -132,6 +132,17 @@ status_t GraphicBufferAllocator::allocate(uint32_t width, uint32_t height, status_t error = mAllocator->allocate(width, height, format, layerCount, usage, 1, stride, handle); + size_t bufSize; + + // if stride has no meaning or is too large, + // approximate size with the input width instead + if ((*stride) != 0 && + std::numeric_limits<size_t>::max() / height / (*stride) < static_cast<size_t>(bpp)) { + bufSize = static_cast<size_t>(width) * height * bpp; + } else { + bufSize = static_cast<size_t>((*stride)) * height * bpp; + } + if (error == NO_ERROR) { Mutex::Autolock _l(sLock); KeyedVector<buffer_handle_t, alloc_rec_t>& list(sAllocList); @@ -142,7 +153,7 @@ status_t GraphicBufferAllocator::allocate(uint32_t width, uint32_t height, rec.format = format; rec.layerCount = layerCount; rec.usage = usage; - rec.size = static_cast<size_t>(height * (*stride) * bpp); + rec.size = bufSize; rec.requestorName = std::move(requestorName); list.add(*handle, rec); diff --git a/libs/ui/include/ui/GraphicBufferAllocator.h b/libs/ui/include/ui/GraphicBufferAllocator.h index 25d4512859..324d9e143b 100644 --- a/libs/ui/include/ui/GraphicBufferAllocator.h +++ b/libs/ui/include/ui/GraphicBufferAllocator.h @@ -54,7 +54,7 @@ public: void dump(std::string& res) const; static void dumpToSystemLog(); -private: +protected: struct alloc_rec_t { uint32_t width; uint32_t height; diff --git a/libs/ui/tests/Android.bp b/libs/ui/tests/Android.bp index 99434b7273..0a07a48bd1 100644 --- a/libs/ui/tests/Android.bp +++ b/libs/ui/tests/Android.bp @@ -29,6 +29,26 @@ cc_test { } cc_test { + name: "GraphicBufferAllocator_test", + header_libs: [ + "libdvr_headers", + "libnativewindow_headers", + ], + static_libs: [ + "libgmock", + ], + shared_libs: [ + "liblog", + "libui", + ], + srcs: [ + "GraphicBufferAllocator_test.cpp", + "mock/MockGrallocAllocator.cpp", + ], + cflags: ["-Wall", "-Werror"], +} + +cc_test { name: "GraphicBuffer_test", header_libs: [ "libdvr_headers", diff --git a/libs/ui/tests/GraphicBufferAllocator_test.cpp b/libs/ui/tests/GraphicBufferAllocator_test.cpp new file mode 100644 index 0000000000..efca083e6e --- /dev/null +++ b/libs/ui/tests/GraphicBufferAllocator_test.cpp @@ -0,0 +1,116 @@ +/* + * Copyright 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "GraphicBufferAllocatorTest" + +#include <ui/GraphicBuffer.h> +#include <ui/GraphicBufferAllocator.h> +#include <ui/PixelFormat.h> + +#include <gtest/gtest.h> + +#include "mock/MockGrallocAllocator.h" + +#include <algorithm> +#include <limits> + +namespace android { + +namespace { + +constexpr uint32_t kTestWidth = 1024; +constexpr uint32_t kTestHeight = 1; +constexpr uint32_t kTestLayerCount = 1; +constexpr uint64_t kTestUsage = GraphicBuffer::USAGE_SW_WRITE_OFTEN; + +} // namespace + +using ::testing::DoAll; +using ::testing::Return; +using ::testing::SetArgPointee; + +class TestableGraphicBufferAllocator : public GraphicBufferAllocator { +public: + TestableGraphicBufferAllocator() { + mAllocator = std::make_unique<const mock::MockGrallocAllocator>(); + } + void setUpAllocateExpectations(status_t err, uint32_t stride) { + std::cout << "Setting expected stride to " << stride << std::endl; + EXPECT_CALL(*(reinterpret_cast<const mock::MockGrallocAllocator*>(mAllocator.get())), + allocate) + .WillOnce(DoAll(SetArgPointee<6>(stride), Return(err))); + } + std::unique_ptr<const GrallocAllocator>& getAllocator() { return mAllocator; } +}; + +class GraphicBufferAllocatorTest : public testing::Test { +public: + GraphicBufferAllocatorTest() : mAllocator() {} + const TestableGraphicBufferAllocator& getAllocator() { return mAllocator; } + +protected: + TestableGraphicBufferAllocator mAllocator; +}; + +TEST_F(GraphicBufferAllocatorTest, AllocateNoError) { + mAllocator.setUpAllocateExpectations(NO_ERROR, kTestWidth); + android::PixelFormat format = PIXEL_FORMAT_RGBA_8888; + uint32_t stride = 0; + buffer_handle_t handle; + status_t err = mAllocator.allocate(kTestWidth, kTestHeight, format, kTestLayerCount, kTestUsage, + &handle, &stride, 0, "GraphicBufferAllocatorTest"); + ASSERT_EQ(NO_ERROR, err); + ASSERT_EQ(kTestWidth, stride); +} + +TEST_F(GraphicBufferAllocatorTest, AllocateZeroStride) { + android::PixelFormat format = PIXEL_FORMAT_RGBA_8888; + uint32_t expectedStride = 0; + + mAllocator.setUpAllocateExpectations(NO_ERROR, expectedStride); + uint32_t stride = 0; + buffer_handle_t handle; + // a divide by zero would cause a crash + status_t err = mAllocator.allocate(kTestWidth, kTestHeight, format, kTestLayerCount, kTestUsage, + &handle, &stride, 0, "GraphicBufferAllocatorTest"); + ASSERT_EQ(NO_ERROR, err); + ASSERT_EQ(expectedStride, stride); +} + +TEST_F(GraphicBufferAllocatorTest, AllocateLargeStride) { + uint32_t height = std::numeric_limits<uint32_t>::max(); + uint32_t bpp = 4; + android::PixelFormat format = PIXEL_FORMAT_RGBA_8888; + + if (std::numeric_limits<size_t>::max() / height / bpp >= std::numeric_limits<uint32_t>::max()) { + std::cout << "stride cannot cause overflow" << std::endl; + GTEST_SUCCEED() << "stride cannot cause overflow"; + return; + } + uint32_t width = std::numeric_limits<size_t>::max() / height / bpp; + + uint32_t expectedStride = std::numeric_limits<uint32_t>::max(); + + mAllocator.setUpAllocateExpectations(NO_ERROR, expectedStride); + uint32_t stride = 0; + buffer_handle_t handle; + // an overflow would cause a crash + status_t err = mAllocator.allocate(width, height, format, kTestLayerCount, kTestUsage, &handle, + &stride, 0, "GraphicBufferAllocatorTest"); + ASSERT_EQ(NO_ERROR, err); + ASSERT_EQ(expectedStride, stride); +} +} // namespace android diff --git a/libs/ui/tests/mock/MockGrallocAllocator.cpp b/libs/ui/tests/mock/MockGrallocAllocator.cpp new file mode 100644 index 0000000000..d71e25fc08 --- /dev/null +++ b/libs/ui/tests/mock/MockGrallocAllocator.cpp @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "MockGrallocAllocator.h" + +namespace android { + +namespace mock { + +MockGrallocAllocator::MockGrallocAllocator() = default; +MockGrallocAllocator::~MockGrallocAllocator() = default; + +} // namespace mock +} // namespace android diff --git a/libs/ui/tests/mock/MockGrallocAllocator.h b/libs/ui/tests/mock/MockGrallocAllocator.h new file mode 100644 index 0000000000..22c80a4638 --- /dev/null +++ b/libs/ui/tests/mock/MockGrallocAllocator.h @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include <gmock/gmock.h> + +#include <ui/Gralloc.h> + +namespace android { + +class GraphicBuffer; + +namespace mock { + +class MockGrallocAllocator : public GrallocAllocator { +public: + MockGrallocAllocator(); + ~MockGrallocAllocator() override; + + MOCK_METHOD(bool, isLoaded, (), (const, override)); + MOCK_METHOD(std::string, dumpDebugInfo, (), (const, override)); + MOCK_METHOD(status_t, allocate, + (uint32_t width, uint32_t height, PixelFormat format, uint32_t layerCount, + uint64_t usage, uint32_t bufferCount, uint32_t* outStride, + buffer_handle_t* outBufferHandles), + (const, override)); +}; + +} // namespace mock +} // namespace android diff --git a/services/surfaceflinger/ContainerLayer.cpp b/services/surfaceflinger/ContainerLayer.cpp index 7927fa95b6..e33bedbfce 100644 --- a/services/surfaceflinger/ContainerLayer.cpp +++ b/services/surfaceflinger/ContainerLayer.cpp @@ -35,11 +35,7 @@ bool ContainerLayer::isVisible() const { return false; } -bool ContainerLayer::canReceiveInput() const { - return !isHiddenByPolicy(); -} void ContainerLayer::setPerFrameData(const sp<const DisplayDevice>&, const ui::Transform&, const Rect&, int32_t, const ui::Dataspace) {} - } // namespace android diff --git a/services/surfaceflinger/ContainerLayer.h b/services/surfaceflinger/ContainerLayer.h index 7222a3e15a..e6dbfcce4e 100644 --- a/services/surfaceflinger/ContainerLayer.h +++ b/services/surfaceflinger/ContainerLayer.h @@ -31,8 +31,6 @@ public: const char* getTypeId() const override { return "ContainerLayer"; } bool isVisible() const override; - bool canReceiveInput() const override; - void setPerFrameData(const sp<const DisplayDevice>& display, const ui::Transform& transform, const Rect& viewport, int32_t supportedPerFrameMetadata, const ui::Dataspace targetDataspace) override; diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index dda15e9e1e..3e6ddedf48 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -2106,7 +2106,7 @@ std::shared_ptr<compositionengine::Layer> Layer::getCompositionLayer() const { } bool Layer::canReceiveInput() const { - return isVisible(); + return !isHiddenByPolicy(); } compositionengine::OutputLayer* Layer::findOutputLayerForDisplay( diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 60b3a11754..dbeae31bcf 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6236,7 +6236,7 @@ status_t SurfaceFlinger::setAllowedDisplayConfigs(const sp<IBinder>& displayToke return NO_ERROR; } - postMessageSync(new LambdaMessage([&]() NO_THREAD_SAFETY_ANALYSIS { + postMessageSync(new LambdaMessage([&]() { const auto display = getDisplayDeviceLocked(displayToken); if (!display) { ALOGE("Attempt to set allowed display configs for invalid display token %p", @@ -6244,6 +6244,7 @@ status_t SurfaceFlinger::setAllowedDisplayConfigs(const sp<IBinder>& displayToke } else if (display->isVirtual()) { ALOGW("Attempt to set allowed display configs for virtual display"); } else { + Mutex::Autolock lock(mStateLock); setAllowedDisplayConfigsInternal(display, allowedConfigs); } })); |