commit 0449b0fa3e3e88da8622da6e95a7eefea8c46a70
author Lloyd Pique <lpique@google.com> Thu Dec 20 16:23:45 2018 -0800
committer Lloyd Pique <lpique@google.com> Mon Jan 07 13:39:45 2019 -0800
tree 57eac962d959508de399454604076cd343d36f66
parent f2c793939c42ff233ced4095cf85bb93ec74523f

Revert "SurfaceFlinger: protect state members in Layer"

State update transactions must be atomic. The fine-grained lock on each
Layer implied otherwise. Revert the fine grained lock as being
unhelpful.

Unfortunately there does not seem to be a way to use Clang thread
annotations to specify the desired locking behavior.

Note that the parent CL addresses the locking problem that led to the
bug.

This reverts commit 83729883eecd31a9907bc79bc21998a90f17105c.

Bug: 119481871
Test: SurfaceFlinger unit tests
Test: go/wm-smoke
Change-Id: I361741f8d10102aeb57f164c847c6063ff93dd14

services/surfaceflinger/BufferLayer.cpp

diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index 0a3be71..4e4d7dd 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -396,7 +396,6 @@
     }
 
     // Capture the old state of the layer for comparisons later
-    Mutex::Autolock lock(mStateMutex);
     const State& s(getDrawingState());
     const bool oldOpacity = isOpaque(s);
     sp<GraphicBuffer> oldBuffer = mActiveBuffer;
@@ -503,7 +502,7 @@
 
     // FIXME: postedRegion should be dirty & bounds
     // transform the dirty region to window-manager space
-    return getTransformLocked().transform(Region(getBufferSize(s)));
+    return getTransform().transform(Region(getBufferSize(s)));
 }
 
 // transaction
@@ -551,7 +550,7 @@
 
 // h/w composer set-up
 bool BufferLayer::allTransactionsSignaled() {
-    auto headFrameNumber = getHeadFrameNumberLocked();
+    auto headFrameNumber = getHeadFrameNumber();
     bool matchingFramesFound = false;
     bool allTransactionsApplied = true;
     Mutex::Autolock lock(mLocalSyncPointMutex);
@@ -604,7 +603,6 @@
 
 void BufferLayer::drawWithOpenGL(const RenderArea& renderArea, bool useIdentityTransform) const {
     ATRACE_CALL();
-    Mutex::Autolock lock(mStateMutex);
     const State& s(getDrawingState());
 
     computeGeometry(renderArea, getBE().mMesh, useIdentityTransform);
@@ -623,9 +621,9 @@
      * minimal value)? Or, we could make GL behave like HWC -- but this feel
      * like more of a hack.
      */
-    const Rect bounds{computeBoundsLocked()}; // Rounds from FloatRect
+    const Rect bounds{computeBounds()}; // Rounds from FloatRect
 
-    ui::Transform t = getTransformLocked();
+    ui::Transform t = getTransform();
     Rect win = bounds;
     const int bufferWidth = getBufferSize(s).getWidth();
     const int bufferHeight = getBufferSize(s).getHeight();
@@ -644,7 +642,7 @@
     texCoords[2] = vec2(right, 1.0f - bottom);
     texCoords[3] = vec2(right, 1.0f - top);
 
-    const auto roundedCornerState = getRoundedCornerStateLocked();
+    const auto roundedCornerState = getRoundedCornerState();
     const auto cropRect = roundedCornerState.cropRect;
     setupRoundedCornersCropCoordinates(win, cropRect);
 
@@ -666,12 +664,7 @@
 }
 
 uint64_t BufferLayer::getHeadFrameNumber() const {
-    Mutex::Autolock lock(mStateMutex);
-    return getHeadFrameNumberLocked();
-}
-
-uint64_t BufferLayer::getHeadFrameNumberLocked() const {
-    if (hasFrameUpdateLocked()) {
+    if (hasFrameUpdate()) {
         return getFrameNumber();
     } else {
         return mCurrentFrameNumber;
@@ -698,7 +691,7 @@
         std::swap(bufWidth, bufHeight);
     }
 
-    if (getTransformToDisplayInverseLocked()) {
+    if (getTransformToDisplayInverse()) {
         uint32_t invTransform = DisplayDevice::getPrimaryDisplayOrientationTransform();
         if (invTransform & ui::Transform::ROT_90) {
             std::swap(bufWidth, bufHeight);