From f9b7f8548eb95f2c387c4c2331acfaa04b6099b8 Mon Sep 17 00:00:00 2001 From: Aravind Akella Date: Thu, 10 Sep 2015 14:52:31 -0700 Subject: [PATCH 01/14] Set DATA_INJECTION mode flag for sensors. Bug: 24001171 Change-Id: I70133546c68fb478b2c2062f05a4164a36cd9e4b --- libs/gui/Sensor.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libs/gui/Sensor.cpp b/libs/gui/Sensor.cpp index 2545eec33..1b8d06e54 100644 --- a/libs/gui/Sensor.cpp +++ b/libs/gui/Sensor.cpp @@ -241,6 +241,11 @@ Sensor::Sensor(struct sensor_t const* hwSensor, int halVersion) break; } + // Set DATA_INJECTION flag here. Defined in HAL 1_4. + if (halVersion >= SENSORS_DEVICE_API_VERSION_1_4) { + mFlags |= (hwSensor->flags & DATA_INJECTION_MASK); + } + // For the newer HALs log errors if reporting mask flags are set incorrectly. if (halVersion >= SENSORS_DEVICE_API_VERSION_1_3) { // Wake-up flag is set here. From c1e6fbb52c3f85cc7610d1d07d12be38f70b4ed4 Mon Sep 17 00:00:00 2001 From: Naveen Leekha Date: Tue, 22 Sep 2015 17:58:21 -0700 Subject: [PATCH 02/14] Initialize local variables to avoid data leak The uninitialized local variables pick up whatever the memory content was there on stack. This data gets sent to the remote process in case of a failed transaction, which is a security issue. Fixed. (Manual merge of master change 12ba0f57d028a9c8f4eb3afddc326b70677d1e0c ) For b/23696300 Change-Id: I665212d10da56f0803b5bb772d14c77e632ba2ab --- libs/gui/IGraphicBufferProducer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp index fc86e608a..b73e69e2b 100644 --- a/libs/gui/IGraphicBufferProducer.cpp +++ b/libs/gui/IGraphicBufferProducer.cpp @@ -201,7 +201,7 @@ status_t BnGraphicBufferProducer::onTransact( uint32_t h = data.readInt32(); uint32_t format = data.readInt32(); uint32_t usage = data.readInt32(); - int buf; + int buf = 0; sp fence; int result = dequeueBuffer(&buf, &fence, async, w, h, format, usage); reply->writeInt32(buf); @@ -233,7 +233,7 @@ status_t BnGraphicBufferProducer::onTransact( } break; case QUERY: { CHECK_INTERFACE(IGraphicBufferProducer, data, reply); - int value; + int value = 0; int what = data.readInt32(); int res = query(what, &value); reply->writeInt32(value); From 115f93eeebf7f33b56ed090de70d6e8c733e5d88 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Thu, 17 Sep 2015 18:04:50 -0700 Subject: [PATCH 03/14] Allow defining replacement key events in keymap Currently keyboard maps allow to assign character sequences to key events and allow specifying a so-called "fallback" key events that are re-injected into input stream if target application indicates that it was not able to handle the original key event. Unfortunately there is no way to perform substitution before handing the event to applicationis. This change adds a new keymap keyword "replace" that allows users query "replacement" actions for key (if any), with the intent that such replacement happens early in the event handling process. Bug: 24504154 Change-Id: I3e6a2476c856524171df00ad22ff56f2018c1278 --- include/input/KeyCharacterMap.h | 8 +++ include/input/Keyboard.h | 7 ++ libs/input/KeyCharacterMap.cpp | 112 +++++++++++++++++++++++++++----- libs/input/Keyboard.cpp | 5 ++ 4 files changed, 117 insertions(+), 15 deletions(-) diff --git a/include/input/KeyCharacterMap.h b/include/input/KeyCharacterMap.h index e70666ab2..3f0914b67 100644 --- a/include/input/KeyCharacterMap.h +++ b/include/input/KeyCharacterMap.h @@ -124,6 +124,11 @@ public: * the mapping in some way. */ status_t mapKey(int32_t scanCode, int32_t usageCode, int32_t* outKeyCode) const; + /* Tries to find a replacement key code for a given key code and meta state + * in character map. */ + void tryRemapKey(int32_t scanCode, int32_t metaState, + int32_t* outKeyCode, int32_t* outMetaState) const; + #if HAVE_ANDROID_OS /* Reads a key map from a parcel. */ static sp readFromParcel(Parcel* parcel); @@ -151,6 +156,9 @@ private: /* The fallback keycode if the key is not handled. */ int32_t fallbackKeyCode; + + /* The replacement keycode if the key has to be replaced outright. */ + int32_t replacementKeyCode; }; struct Key { diff --git a/include/input/Keyboard.h b/include/input/Keyboard.h index 519bb22d2..d4903e98d 100644 --- a/include/input/Keyboard.h +++ b/include/input/Keyboard.h @@ -87,6 +87,13 @@ extern bool isEligibleBuiltInKeyboard(const InputDeviceIdentifier& deviceIdentif */ extern int32_t updateMetaState(int32_t keyCode, bool down, int32_t oldMetaState); +/** + * Normalizes the meta state such that if either the left or right modifier + * meta state bits are set then the result will also include the universal + * bit for that modifier. + */ +extern int32_t normalizeMetaState(int32_t oldMetaState); + /** * Returns true if a key is a meta key like ALT or CAPS_LOCK. */ diff --git a/libs/input/KeyCharacterMap.cpp b/libs/input/KeyCharacterMap.cpp index b03e01eff..df3f0dd1d 100644 --- a/libs/input/KeyCharacterMap.cpp +++ b/libs/input/KeyCharacterMap.cpp @@ -332,33 +332,75 @@ status_t KeyCharacterMap::mapKey(int32_t scanCode, int32_t usageCode, int32_t* o if (usageCode) { ssize_t index = mKeysByUsageCode.indexOfKey(usageCode); if (index >= 0) { -#if DEBUG_MAPPING - ALOGD("mapKey: scanCode=%d, usageCode=0x%08x ~ Result keyCode=%d.", - scanCode, usageCode, *outKeyCode); -#endif *outKeyCode = mKeysByUsageCode.valueAt(index); +#if DEBUG_MAPPING + ALOGD("mapKey: scanCode=%d, usageCode=0x%08x ~ Result keyCode=%d.", + scanCode, usageCode, *outKeyCode); +#endif return OK; } } if (scanCode) { ssize_t index = mKeysByScanCode.indexOfKey(scanCode); if (index >= 0) { -#if DEBUG_MAPPING - ALOGD("mapKey: scanCode=%d, usageCode=0x%08x ~ Result keyCode=%d.", - scanCode, usageCode, *outKeyCode); -#endif *outKeyCode = mKeysByScanCode.valueAt(index); +#if DEBUG_MAPPING + ALOGD("mapKey: scanCode=%d, usageCode=0x%08x ~ Result keyCode=%d.", + scanCode, usageCode, *outKeyCode); +#endif return OK; } } #if DEBUG_MAPPING - ALOGD("mapKey: scanCode=%d, usageCode=0x%08x ~ Failed.", scanCode, usageCode); + ALOGD("mapKey: scanCode=%d, usageCode=0x%08x ~ Failed.", scanCode, usageCode); #endif *outKeyCode = AKEYCODE_UNKNOWN; return NAME_NOT_FOUND; } +void KeyCharacterMap::tryRemapKey(int32_t keyCode, int32_t metaState, + int32_t *outKeyCode, int32_t *outMetaState) const { + *outKeyCode = keyCode; + *outMetaState = metaState; + + const Key* key; + const Behavior* behavior; + if (getKeyBehavior(keyCode, metaState, &key, &behavior)) { + if (behavior->replacementKeyCode) { + *outKeyCode = behavior->replacementKeyCode; + int32_t newMetaState = metaState & ~behavior->metaState; + // Reset dependent meta states. + if (behavior->metaState & AMETA_ALT_ON) { + newMetaState &= ~(AMETA_ALT_LEFT_ON | AMETA_ALT_RIGHT_ON); + } + if (behavior->metaState & (AMETA_ALT_LEFT_ON | AMETA_ALT_RIGHT_ON)) { + newMetaState &= ~AMETA_ALT_ON; + } + if (behavior->metaState & AMETA_CTRL_ON) { + newMetaState &= ~(AMETA_CTRL_LEFT_ON | AMETA_CTRL_RIGHT_ON); + } + if (behavior->metaState & (AMETA_CTRL_LEFT_ON | AMETA_CTRL_RIGHT_ON)) { + newMetaState &= ~AMETA_CTRL_ON; + } + if (behavior->metaState & AMETA_SHIFT_ON) { + newMetaState &= ~(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_RIGHT_ON); + } + if (behavior->metaState & (AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_RIGHT_ON)) { + newMetaState &= ~AMETA_SHIFT_ON; + } + // ... and put universal bits back if needed + *outMetaState = normalizeMetaState(newMetaState); + } + } + +#if DEBUG_MAPPING + ALOGD("tryRemapKey: keyCode=%d, metaState=0x%08x ~ " + "replacement keyCode=%d, replacement metaState=0x%08x.", + keyCode, metaState, *outKeyCode, *outMetaState); +#endif +} + bool KeyCharacterMap::getKey(int32_t keyCode, const Key** outKey) const { ssize_t index = mKeys.indexOfKey(keyCode); if (index >= 0) { @@ -584,6 +626,7 @@ sp KeyCharacterMap::readFromParcel(Parcel* parcel) { int32_t metaState = parcel->readInt32(); char16_t character = parcel->readInt32(); int32_t fallbackKeyCode = parcel->readInt32(); + int32_t replacementKeyCode = parcel->readInt32(); if (parcel->errorCheck()) { return NULL; } @@ -592,6 +635,7 @@ sp KeyCharacterMap::readFromParcel(Parcel* parcel) { behavior->metaState = metaState; behavior->character = character; behavior->fallbackKeyCode = fallbackKeyCode; + behavior->replacementKeyCode = replacementKeyCode; if (lastBehavior) { lastBehavior->next = behavior; } else { @@ -624,6 +668,7 @@ void KeyCharacterMap::writeToParcel(Parcel* parcel) const { parcel->writeInt32(behavior->metaState); parcel->writeInt32(behavior->character); parcel->writeInt32(behavior->fallbackKeyCode); + parcel->writeInt32(behavior->replacementKeyCode); } parcel->writeInt32(0); } @@ -655,13 +700,14 @@ KeyCharacterMap::Key::~Key() { // --- KeyCharacterMap::Behavior --- KeyCharacterMap::Behavior::Behavior() : - next(NULL), metaState(0), character(0), fallbackKeyCode(0) { + next(NULL), metaState(0), character(0), fallbackKeyCode(0), replacementKeyCode(0) { } KeyCharacterMap::Behavior::Behavior(const Behavior& other) : next(other.next ? new Behavior(*other.next) : NULL), metaState(other.metaState), character(other.character), - fallbackKeyCode(other.fallbackKeyCode) { + fallbackKeyCode(other.fallbackKeyCode), + replacementKeyCode(other.replacementKeyCode) { } @@ -923,6 +969,7 @@ status_t KeyCharacterMap::Parser::parseKeyProperty() { Behavior behavior; bool haveCharacter = false; bool haveFallback = false; + bool haveReplacement = false; do { char ch = mTokenizer->peekChar(); @@ -939,6 +986,11 @@ status_t KeyCharacterMap::Parser::parseKeyProperty() { mTokenizer->getLocation().string()); return BAD_VALUE; } + if (haveReplacement) { + ALOGE("%s: Cannot combine character literal with replace action.", + mTokenizer->getLocation().string()); + return BAD_VALUE; + } behavior.character = character; haveCharacter = true; } else { @@ -949,6 +1001,11 @@ status_t KeyCharacterMap::Parser::parseKeyProperty() { mTokenizer->getLocation().string()); return BAD_VALUE; } + if (haveReplacement) { + ALOGE("%s: Cannot combine 'none' with replace action.", + mTokenizer->getLocation().string()); + return BAD_VALUE; + } haveCharacter = true; } else if (token == "fallback") { mTokenizer->skipDelimiters(WHITESPACE); @@ -960,13 +1017,36 @@ status_t KeyCharacterMap::Parser::parseKeyProperty() { token.string()); return BAD_VALUE; } - if (haveFallback) { - ALOGE("%s: Cannot combine multiple fallback key codes.", + if (haveFallback || haveReplacement) { + ALOGE("%s: Cannot combine multiple fallback/replacement key codes.", mTokenizer->getLocation().string()); return BAD_VALUE; } behavior.fallbackKeyCode = keyCode; haveFallback = true; + } else if (token == "replace") { + mTokenizer->skipDelimiters(WHITESPACE); + token = mTokenizer->nextToken(WHITESPACE); + int32_t keyCode = getKeyCodeByLabel(token.string()); + if (!keyCode) { + ALOGE("%s: Invalid key code label for replace, got '%s'.", + mTokenizer->getLocation().string(), + token.string()); + return BAD_VALUE; + } + if (haveCharacter) { + ALOGE("%s: Cannot combine character literal with replace action.", + mTokenizer->getLocation().string()); + return BAD_VALUE; + } + if (haveFallback || haveReplacement) { + ALOGE("%s: Cannot combine multiple fallback/replacement key codes.", + mTokenizer->getLocation().string()); + return BAD_VALUE; + } + behavior.replacementKeyCode = keyCode; + haveReplacement = true; + } else { ALOGE("%s: Expected a key behavior after ':'.", mTokenizer->getLocation().string()); @@ -1016,8 +1096,10 @@ status_t KeyCharacterMap::Parser::parseKeyProperty() { newBehavior->next = key->firstBehavior; key->firstBehavior = newBehavior; #if DEBUG_PARSER - ALOGD("Parsed key meta: keyCode=%d, meta=0x%x, char=%d, fallback=%d.", mKeyCode, - newBehavior->metaState, newBehavior->character, newBehavior->fallbackKeyCode); + ALOGD("Parsed key meta: keyCode=%d, meta=0x%x, char=%d, fallback=%d replace=%d.", + mKeyCode, + newBehavior->metaState, newBehavior->character, + newBehavior->fallbackKeyCode, newBehavior->replacementKeyCode); #endif break; } diff --git a/libs/input/Keyboard.cpp b/libs/input/Keyboard.cpp index f4d95077d..9a01395da 100644 --- a/libs/input/Keyboard.cpp +++ b/libs/input/Keyboard.cpp @@ -176,6 +176,11 @@ static int32_t setEphemeralMetaState(int32_t mask, bool down, int32_t oldMetaSta ~(mask | AMETA_ALT_ON | AMETA_SHIFT_ON | AMETA_CTRL_ON | AMETA_META_ON); } + return normalizeMetaState(newMetaState); +} + +int32_t normalizeMetaState(int32_t oldMetaState) { + int32_t newMetaState = oldMetaState; if (newMetaState & (AMETA_ALT_LEFT_ON | AMETA_ALT_RIGHT_ON)) { newMetaState |= AMETA_ALT_ON; } From 0faaa0bd7aa5dadea7c365fbb1f186da6eb097ef Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Thu, 24 Sep 2015 13:13:55 -0700 Subject: [PATCH 04/14] Inputflinger: hook up key event replacement processing Add handling of "replacement" key events in InputReader and EventHub by consulting device's character key map (if exists) for presence of replacement key code for given get code and meta state combination, before passing it to InputDispatcher. This enables defining special keys, such as ESC, on keyboards lacking enough physical keys, via combination of normal keys and modifiers, for example AltR + 1 => ESC. Bug: 24504154 Change-Id: I7e36104808bedcf724436c1fbb63d37c35cca8af --- services/inputflinger/EventHub.cpp | 30 +++++++++++----- services/inputflinger/EventHub.h | 10 +++--- services/inputflinger/InputReader.cpp | 34 +++++++++++++------ services/inputflinger/InputReader.h | 3 +- .../inputflinger/tests/InputReader_test.cpp | 8 +++-- 5 files changed, 58 insertions(+), 27 deletions(-) diff --git a/services/inputflinger/EventHub.cpp b/services/inputflinger/EventHub.cpp index 6b60c7cf7..585960628 100644 --- a/services/inputflinger/EventHub.cpp +++ b/services/inputflinger/EventHub.cpp @@ -438,10 +438,12 @@ bool EventHub::markSupportedKeyCodes(int32_t deviceId, size_t numCodes, return false; } -status_t EventHub::mapKey(int32_t deviceId, int32_t scanCode, int32_t usageCode, - int32_t* outKeycode, uint32_t* outFlags) const { +status_t EventHub::mapKey(int32_t deviceId, + int32_t scanCode, int32_t usageCode, int32_t metaState, + int32_t* outKeycode, int32_t* outMetaState, uint32_t* outFlags) const { AutoMutex _l(mLock); Device* device = getDeviceLocked(deviceId); + status_t status = NAME_NOT_FOUND; if (device) { // Check the key character map first. @@ -449,22 +451,34 @@ status_t EventHub::mapKey(int32_t deviceId, int32_t scanCode, int32_t usageCode, if (kcm != NULL) { if (!kcm->mapKey(scanCode, usageCode, outKeycode)) { *outFlags = 0; - return NO_ERROR; + status = NO_ERROR; } } // Check the key layout next. - if (device->keyMap.haveKeyLayout()) { + if (status != NO_ERROR && device->keyMap.haveKeyLayout()) { if (!device->keyMap.keyLayoutMap->mapKey( scanCode, usageCode, outKeycode, outFlags)) { - return NO_ERROR; + status = NO_ERROR; + } + } + + if (status == NO_ERROR) { + if (kcm != NULL) { + kcm->tryRemapKey(*outKeycode, metaState, outKeycode, outMetaState); + } else { + *outMetaState = metaState; } } } - *outKeycode = 0; - *outFlags = 0; - return NAME_NOT_FOUND; + if (status != NO_ERROR) { + *outKeycode = 0; + *outFlags = 0; + *outMetaState = metaState; + } + + return status; } status_t EventHub::mapAxis(int32_t deviceId, int32_t scanCode, AxisInfo* outAxisInfo) const { diff --git a/services/inputflinger/EventHub.h b/services/inputflinger/EventHub.h index 3ec49105c..0f94c770e 100644 --- a/services/inputflinger/EventHub.h +++ b/services/inputflinger/EventHub.h @@ -197,8 +197,9 @@ public: virtual bool hasInputProperty(int32_t deviceId, int property) const = 0; - virtual status_t mapKey(int32_t deviceId, int32_t scanCode, int32_t usageCode, - int32_t* outKeycode, uint32_t* outFlags) const = 0; + virtual status_t mapKey(int32_t deviceId, + int32_t scanCode, int32_t usageCode, int32_t metaState, + int32_t* outKeycode, int32_t *outMetaState, uint32_t* outFlags) const = 0; virtual status_t mapAxis(int32_t deviceId, int32_t scanCode, AxisInfo* outAxisInfo) const = 0; @@ -285,8 +286,9 @@ public: virtual bool hasInputProperty(int32_t deviceId, int property) const; - virtual status_t mapKey(int32_t deviceId, int32_t scanCode, int32_t usageCode, - int32_t* outKeycode, uint32_t* outFlags) const; + virtual status_t mapKey(int32_t deviceId, + int32_t scanCode, int32_t usageCode, int32_t metaState, + int32_t* outKeycode, int32_t *outMetaState, uint32_t* outFlags) const; virtual status_t mapAxis(int32_t deviceId, int32_t scanCode, AxisInfo* outAxisInfo) const; diff --git a/services/inputflinger/InputReader.cpp b/services/inputflinger/InputReader.cpp index 36095bf1e..b2cbfe801 100644 --- a/services/inputflinger/InputReader.cpp +++ b/services/inputflinger/InputReader.cpp @@ -2177,13 +2177,7 @@ void KeyboardInputMapper::process(const RawEvent* rawEvent) { mCurrentHidUsage = 0; if (isKeyboardOrGamepadKey(scanCode)) { - int32_t keyCode; - uint32_t flags; - if (getEventHub()->mapKey(getDeviceId(), scanCode, usageCode, &keyCode, &flags)) { - keyCode = AKEYCODE_UNKNOWN; - flags = 0; - } - processKey(rawEvent->when, rawEvent->value != 0, keyCode, scanCode, flags); + processKey(rawEvent->when, rawEvent->value != 0, scanCode, usageCode); } break; } @@ -2208,8 +2202,18 @@ bool KeyboardInputMapper::isKeyboardOrGamepadKey(int32_t scanCode) { || (scanCode >= BTN_JOYSTICK && scanCode < BTN_DIGI); } -void KeyboardInputMapper::processKey(nsecs_t when, bool down, int32_t keyCode, - int32_t scanCode, uint32_t policyFlags) { +void KeyboardInputMapper::processKey(nsecs_t when, bool down, int32_t scanCode, + int32_t usageCode) { + int32_t keyCode; + int32_t keyMetaState; + uint32_t policyFlags; + + if (getEventHub()->mapKey(getDeviceId(), scanCode, usageCode, mMetaState, + &keyCode, &keyMetaState, &policyFlags)) { + keyCode = AKEYCODE_UNKNOWN; + keyMetaState = mMetaState; + policyFlags = 0; + } if (down) { // Rotate key codes according to orientation if needed. @@ -2262,6 +2266,12 @@ void KeyboardInputMapper::processKey(nsecs_t when, bool down, int32_t keyCode, if (metaStateChanged) { mMetaState = newMetaState; updateLedState(false); + + // If global meta state changed send it along with the key. + // If it has not changed then we'll use what keymap gave us, + // since key replacement logic might temporarily reset a few + // meta bits for given key. + keyMetaState = newMetaState; } nsecs_t downTime = mDownTime; @@ -2289,7 +2299,7 @@ void KeyboardInputMapper::processKey(nsecs_t when, bool down, int32_t keyCode, NotifyKeyArgs args(when, getDeviceId(), mSource, policyFlags, down ? AKEY_EVENT_ACTION_DOWN : AKEY_EVENT_ACTION_UP, - AKEY_EVENT_FLAG_FROM_SYSTEM, keyCode, scanCode, newMetaState, downTime); + AKEY_EVENT_FLAG_FROM_SYSTEM, keyCode, scanCode, keyMetaState, downTime); getListener()->notifyKey(&args); } @@ -3543,8 +3553,10 @@ void TouchInputMapper::configureVirtualKeys() { virtualKey.scanCode = virtualKeyDefinition.scanCode; int32_t keyCode; + int32_t dummyKeyMetaState; uint32_t flags; - if (getEventHub()->mapKey(getDeviceId(), virtualKey.scanCode, 0, &keyCode, &flags)) { + if (getEventHub()->mapKey(getDeviceId(), virtualKey.scanCode, 0, 0, + &keyCode, &dummyKeyMetaState, &flags)) { ALOGW(INDENT "VirtualKey %d: could not obtain key code, ignoring", virtualKey.scanCode); mVirtualKeys.pop(); // drop the key diff --git a/services/inputflinger/InputReader.h b/services/inputflinger/InputReader.h index 7cb4680ce..30c84b126 100644 --- a/services/inputflinger/InputReader.h +++ b/services/inputflinger/InputReader.h @@ -1154,8 +1154,7 @@ private: bool isKeyboardOrGamepadKey(int32_t scanCode); - void processKey(nsecs_t when, bool down, int32_t keyCode, int32_t scanCode, - uint32_t policyFlags); + void processKey(nsecs_t when, bool down, int32_t scanCode, int32_t usageCode); ssize_t findKeyDown(int32_t scanCode); diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp index f34b810f1..42bc8656b 100644 --- a/services/inputflinger/tests/InputReader_test.cpp +++ b/services/inputflinger/tests/InputReader_test.cpp @@ -518,8 +518,9 @@ private: return false; } - virtual status_t mapKey(int32_t deviceId, int32_t scanCode, int32_t usageCode, - int32_t* outKeycode, uint32_t* outFlags) const { + virtual status_t mapKey(int32_t deviceId, + int32_t scanCode, int32_t usageCode, int32_t metaState, + int32_t* outKeycode, int32_t *outMetaState, uint32_t* outFlags) const { Device* device = getDevice(deviceId); if (device) { const KeyInfo* key = getKey(device, scanCode, usageCode); @@ -530,6 +531,9 @@ private: if (outFlags) { *outFlags = key->flags; } + if (outMetaState) { + *outMetaState = metaState; + } return OK; } } From 552a8a5d8df32f659b8d11311a244cdc6d3b7733 Mon Sep 17 00:00:00 2001 From: Flanker Date: Mon, 7 Sep 2015 15:28:58 +0800 Subject: [PATCH 05/14] add number constraint for samples per MotionEvent Bug:23905002 Change-Id: Ifd24802977c3dcdd1dbc5120a78aac41beae4603 Signed-off-by: Adam Lesinski --- include/input/Input.h | 5 +++++ libs/input/Input.cpp | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/include/input/Input.h b/include/input/Input.h index 4a67f47f9..92b3d3695 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -110,6 +110,11 @@ enum { */ #define MAX_POINTERS 16 +/* + * Maximum number of samples supported per motion event. + */ +#define MAX_SAMPLES UINT16_MAX + /* * Maximum pointer id value supported in a motion event. * Smallest pointer id is 0. diff --git a/libs/input/Input.cpp b/libs/input/Input.cpp index 2c1418e07..b64cb2ca9 100644 --- a/libs/input/Input.cpp +++ b/libs/input/Input.cpp @@ -424,7 +424,8 @@ void MotionEvent::transform(const float matrix[9]) { status_t MotionEvent::readFromParcel(Parcel* parcel) { size_t pointerCount = parcel->readInt32(); size_t sampleCount = parcel->readInt32(); - if (pointerCount == 0 || pointerCount > MAX_POINTERS || sampleCount == 0) { + if (pointerCount == 0 || pointerCount > MAX_POINTERS || + sampleCount == 0 || sampleCount > MAX_SAMPLES) { return BAD_VALUE; } From 312d7555cb71ce7fb73bc758b9e30653e223b2f3 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Wed, 14 Oct 2015 11:10:24 -0700 Subject: [PATCH 06/14] egl: Remove window disconnect before calling driver eglDestroySurface This was originally added for b/14445579. An in-development app was attempting to render to a window as an EGLSurface, then tear that down, change some window properties, and create a new EGLSurface. The second eglCreateWindowSurface failed because the window was already connected. This change went in, but it turned out the real problem was that the app still (unintentionally) had the surface current. After the app bug was fixed, nobody revisited whether this change was actually needed. Turns out it wasn't needed. After an EGLSurface is both destroyed *AND* not current (basically refcount==0), we were already disconnecting the window in ~egl_surface_t(). Apart from being unnecessary and redundant, disconnecting the window here is wrong for two reasons: (a) The surface may still be in use after eglDestroySurface, if it was still current. Rendering is undefined in that case, but disconnecting the window leads to more catastrophic results than necessary. (b) It's being called before calling the driver's eglDestroySurface. The driver will almost definitely have a buffer dequeued that it needs to cancel, and by disconnecting first we turn that into an error that they don't have a graceful way to deal with. Bug: 24524053 Change-Id: Ib063134413d25d3526f794aafb5e333e3417ea42 --- opengl/libs/EGL/eglApi.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/opengl/libs/EGL/eglApi.cpp b/opengl/libs/EGL/eglApi.cpp index 837890759..cdec565e4 100644 --- a/opengl/libs/EGL/eglApi.cpp +++ b/opengl/libs/EGL/eglApi.cpp @@ -595,15 +595,6 @@ EGLBoolean eglDestroySurface(EGLDisplay dpy, EGLSurface surface) return setError(EGL_BAD_SURFACE, EGL_FALSE); egl_surface_t * const s = get_surface(surface); - ANativeWindow* window = s->win.get(); - if (window) { - int result = native_window_api_disconnect(window, NATIVE_WINDOW_API_EGL); - if (result != OK) { - ALOGE("eglDestroySurface: native_window_api_disconnect (win=%p) " - "failed (%#x)", - window, result); - } - } EGLBoolean result = s->cnx->egl.eglDestroySurface(dp->disp.dpy, s->surface); if (result == EGL_TRUE) { _s.terminate(); From 5d17838adef13062717322e79d4db0b9bb6b2395 Mon Sep 17 00:00:00 2001 From: Flanker Date: Mon, 7 Sep 2015 15:28:58 +0800 Subject: [PATCH 07/14] add number constraint for samples per MotionEvent Bug:23905002 Signed-off-by: Adam Lesinski (cherry picked from commit 552a8a5d8df32f659b8d11311a244cdc6d3b7733) Change-Id: I9b7ea859889b7697bee4165a2746602212120543 --- include/input/Input.h | 5 +++++ libs/input/Input.cpp | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/include/input/Input.h b/include/input/Input.h index e77807634..97101a771 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -81,6 +81,11 @@ enum { */ #define MAX_POINTERS 16 +/* + * Maximum number of samples supported per motion event. + */ +#define MAX_SAMPLES UINT16_MAX + /* * Maximum pointer id value supported in a motion event. * Smallest pointer id is 0. diff --git a/libs/input/Input.cpp b/libs/input/Input.cpp index 6f53996b4..83e241c4e 100644 --- a/libs/input/Input.cpp +++ b/libs/input/Input.cpp @@ -490,7 +490,8 @@ void MotionEvent::transform(const float matrix[9]) { status_t MotionEvent::readFromParcel(Parcel* parcel) { size_t pointerCount = parcel->readInt32(); size_t sampleCount = parcel->readInt32(); - if (pointerCount == 0 || pointerCount > MAX_POINTERS || sampleCount == 0) { + if (pointerCount == 0 || pointerCount > MAX_POINTERS || + sampleCount == 0 || sampleCount > MAX_SAMPLES) { return BAD_VALUE; } From e2f499fb734bc30a1e1c947112caa0727349b6ed Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Tue, 20 Oct 2015 19:11:23 -0700 Subject: [PATCH 08/14] Track ashmem memory usage in Parcel Bug: 25004154 Change-Id: Id9d5656dd0605f1b50525596b75601309f67ebdc --- include/binder/Parcel.h | 6 ++++-- libs/binder/Parcel.cpp | 38 ++++++++++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/include/binder/Parcel.h b/include/binder/Parcel.h index 3ada1e909..220a9353f 100644 --- a/include/binder/Parcel.h +++ b/include/binder/Parcel.h @@ -342,9 +342,11 @@ public: private: size_t mBlobAshmemSize; + size_t mOpenAshmemSize; public: size_t getBlobAshmemSize() const; + size_t getOpenAshmemSize() const; }; // --------------------------------------------------------------------------- @@ -412,9 +414,9 @@ inline TextOutput& operator<<(TextOutput& to, const Parcel& parcel) // Generic acquire and release of objects. void acquire_object(const sp& proc, - const flat_binder_object& obj, const void* who); + const flat_binder_object& obj, const void* who, size_t* outAshmemSize); void release_object(const sp& proc, - const flat_binder_object& obj, const void* who); + const flat_binder_object& obj, const void* who, size_t* outAshmemSize); void flatten_binder(const sp& proc, const sp& binder, flat_binder_object* out); diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 7a4ddc43a..1c0358518 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -96,7 +96,7 @@ enum { }; void acquire_object(const sp& proc, - const flat_binder_object& obj, const void* who) + const flat_binder_object& obj, const void* who, size_t* outAshmemSize) { switch (obj.type) { case BINDER_TYPE_BINDER: @@ -123,8 +123,13 @@ void acquire_object(const sp& proc, return; } case BINDER_TYPE_FD: { - // intentionally blank -- nothing to do to acquire this, but we do - // recognize it as a legitimate object type. + if (obj.cookie != 0) { + // If we own an ashmem fd, keep track of how much memory it refers to. + int size = ashmem_get_size_region(obj.handle); + if (size > 0) { + *outAshmemSize += size; + } + } return; } } @@ -133,7 +138,7 @@ void acquire_object(const sp& proc, } void release_object(const sp& proc, - const flat_binder_object& obj, const void* who) + const flat_binder_object& obj, const void* who, size_t* outAshmemSize) { switch (obj.type) { case BINDER_TYPE_BINDER: @@ -160,7 +165,14 @@ void release_object(const sp& proc, return; } case BINDER_TYPE_FD: { - if (obj.cookie != 0) close(obj.handle); + if (obj.cookie != 0) { + int size = ashmem_get_size_region(obj.handle); + if (size > 0) { + *outAshmemSize -= size; + } + + close(obj.handle); + } return; } } @@ -504,7 +516,7 @@ status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) flat_binder_object* flat = reinterpret_cast(mData + off); - acquire_object(proc, *flat, this); + acquire_object(proc, *flat, this, &mOpenAshmemSize); if (flat->type == BINDER_TYPE_FD) { // If this is a file descriptor, we need to dup it so the @@ -1026,7 +1038,7 @@ restart_write: // Need to write meta-data? if (nullMetaData || val.binder != 0) { mObjects[mObjectsSize] = mDataPos; - acquire_object(ProcessState::self(), val, this); + acquire_object(ProcessState::self(), val, this, &mOpenAshmemSize); mObjectsSize++; } @@ -1609,7 +1621,7 @@ void Parcel::releaseObjects() i--; const flat_binder_object* flat = reinterpret_cast(data+objects[i]); - release_object(proc, *flat, this); + release_object(proc, *flat, this, &mOpenAshmemSize); } } @@ -1623,7 +1635,7 @@ void Parcel::acquireObjects() i--; const flat_binder_object* flat = reinterpret_cast(data+objects[i]); - acquire_object(proc, *flat, this); + acquire_object(proc, *flat, this, &mOpenAshmemSize); } } @@ -1805,7 +1817,7 @@ status_t Parcel::continueWrite(size_t desired) // will need to rescan because we may have lopped off the only FDs mFdsKnown = false; } - release_object(proc, *flat, this); + release_object(proc, *flat, this, &mOpenAshmemSize); } binder_size_t* objects = (binder_size_t*)realloc(mObjects, objectsSize*sizeof(binder_size_t)); @@ -1891,6 +1903,7 @@ void Parcel::initState() mAllowFds = true; mOwner = NULL; mBlobAshmemSize = 0; + mOpenAshmemSize = 0; } void Parcel::scanForFds() const @@ -1913,6 +1926,11 @@ size_t Parcel::getBlobAshmemSize() const return mBlobAshmemSize; } +size_t Parcel::getOpenAshmemSize() const +{ + return mOpenAshmemSize; +} + // --- Parcel::Blob --- Parcel::Blob::Blob() : From 834ac204ce52d874cdd7ae921e5a4d85c5e42c52 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Thu, 22 Oct 2015 07:09:23 -0700 Subject: [PATCH 09/14] DO NOT MERGE: fix build breakage fix klp-dev only build breakage. frameworks/native/libs/input/Input.cpp: In member function 'android::status_t android::MotionEvent::readFromParcel(android::Parcel*)': frameworks/native/libs/input/Input.cpp:494:47: error: 'UINT16_MAX' was not declared in this scope Bug: 23905002 Change-Id: I4b6b864ca64d39a8873d045a61e0ddaea2ab9109 --- include/input/Input.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/input/Input.h b/include/input/Input.h index 97101a771..f8075e44c 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -27,6 +27,7 @@ #include #include #include +#include /* * Additional private constants not defined in ndk/ui/input.h. From 778b6f4902ad824d5fc62071caaa837bb47deee5 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Thu, 22 Oct 2015 14:48:50 -0700 Subject: [PATCH 10/14] DO NOT MERGE: fix build try #2 On klp-dev, UINT16_MAX isn't available unless __STDINT_LIMITS is defined, which it's not for this code. This isn't relevant for later branches due to bionic commit e2a292d278b94fec3d078b1f1b27c1f89942c276 Don't use UINT16_MAX when we can just hardcode 65535. Bug: 23905002 Change-Id: Ia1fd0f749cb7a4d19866075abc28ed6960424e54 --- include/input/Input.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/input/Input.h b/include/input/Input.h index f8075e44c..44e40273b 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -85,7 +85,7 @@ enum { /* * Maximum number of samples supported per motion event. */ -#define MAX_SAMPLES UINT16_MAX +#define MAX_SAMPLES 65535 /* * Maximum pointer id value supported in a motion event. From 6880307e8e35a6c484942443fb4ddd6173126152 Mon Sep 17 00:00:00 2001 From: Ian Pedowitz Date: Thu, 22 Oct 2015 22:08:10 +0000 Subject: [PATCH 11/14] Revert "Track ashmem memory usage in Parcel" This reverts commit e2f499fb734bc30a1e1c947112caa0727349b6ed. Bug: 25169267 Bug: 25191602 Bug: 25004154 Change-Id: I24bb0da4e8739ee5a0c251e4adac9904827144e0 --- include/binder/Parcel.h | 6 ++---- libs/binder/Parcel.cpp | 38 ++++++++++---------------------------- 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/include/binder/Parcel.h b/include/binder/Parcel.h index 220a9353f..3ada1e909 100644 --- a/include/binder/Parcel.h +++ b/include/binder/Parcel.h @@ -342,11 +342,9 @@ public: private: size_t mBlobAshmemSize; - size_t mOpenAshmemSize; public: size_t getBlobAshmemSize() const; - size_t getOpenAshmemSize() const; }; // --------------------------------------------------------------------------- @@ -414,9 +412,9 @@ inline TextOutput& operator<<(TextOutput& to, const Parcel& parcel) // Generic acquire and release of objects. void acquire_object(const sp& proc, - const flat_binder_object& obj, const void* who, size_t* outAshmemSize); + const flat_binder_object& obj, const void* who); void release_object(const sp& proc, - const flat_binder_object& obj, const void* who, size_t* outAshmemSize); + const flat_binder_object& obj, const void* who); void flatten_binder(const sp& proc, const sp& binder, flat_binder_object* out); diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 1c0358518..7a4ddc43a 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -96,7 +96,7 @@ enum { }; void acquire_object(const sp& proc, - const flat_binder_object& obj, const void* who, size_t* outAshmemSize) + const flat_binder_object& obj, const void* who) { switch (obj.type) { case BINDER_TYPE_BINDER: @@ -123,13 +123,8 @@ void acquire_object(const sp& proc, return; } case BINDER_TYPE_FD: { - if (obj.cookie != 0) { - // If we own an ashmem fd, keep track of how much memory it refers to. - int size = ashmem_get_size_region(obj.handle); - if (size > 0) { - *outAshmemSize += size; - } - } + // intentionally blank -- nothing to do to acquire this, but we do + // recognize it as a legitimate object type. return; } } @@ -138,7 +133,7 @@ void acquire_object(const sp& proc, } void release_object(const sp& proc, - const flat_binder_object& obj, const void* who, size_t* outAshmemSize) + const flat_binder_object& obj, const void* who) { switch (obj.type) { case BINDER_TYPE_BINDER: @@ -165,14 +160,7 @@ void release_object(const sp& proc, return; } case BINDER_TYPE_FD: { - if (obj.cookie != 0) { - int size = ashmem_get_size_region(obj.handle); - if (size > 0) { - *outAshmemSize -= size; - } - - close(obj.handle); - } + if (obj.cookie != 0) close(obj.handle); return; } } @@ -516,7 +504,7 @@ status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) flat_binder_object* flat = reinterpret_cast(mData + off); - acquire_object(proc, *flat, this, &mOpenAshmemSize); + acquire_object(proc, *flat, this); if (flat->type == BINDER_TYPE_FD) { // If this is a file descriptor, we need to dup it so the @@ -1038,7 +1026,7 @@ restart_write: // Need to write meta-data? if (nullMetaData || val.binder != 0) { mObjects[mObjectsSize] = mDataPos; - acquire_object(ProcessState::self(), val, this, &mOpenAshmemSize); + acquire_object(ProcessState::self(), val, this); mObjectsSize++; } @@ -1621,7 +1609,7 @@ void Parcel::releaseObjects() i--; const flat_binder_object* flat = reinterpret_cast(data+objects[i]); - release_object(proc, *flat, this, &mOpenAshmemSize); + release_object(proc, *flat, this); } } @@ -1635,7 +1623,7 @@ void Parcel::acquireObjects() i--; const flat_binder_object* flat = reinterpret_cast(data+objects[i]); - acquire_object(proc, *flat, this, &mOpenAshmemSize); + acquire_object(proc, *flat, this); } } @@ -1817,7 +1805,7 @@ status_t Parcel::continueWrite(size_t desired) // will need to rescan because we may have lopped off the only FDs mFdsKnown = false; } - release_object(proc, *flat, this, &mOpenAshmemSize); + release_object(proc, *flat, this); } binder_size_t* objects = (binder_size_t*)realloc(mObjects, objectsSize*sizeof(binder_size_t)); @@ -1903,7 +1891,6 @@ void Parcel::initState() mAllowFds = true; mOwner = NULL; mBlobAshmemSize = 0; - mOpenAshmemSize = 0; } void Parcel::scanForFds() const @@ -1926,11 +1913,6 @@ size_t Parcel::getBlobAshmemSize() const return mBlobAshmemSize; } -size_t Parcel::getOpenAshmemSize() const -{ - return mOpenAshmemSize; -} - // --- Parcel::Blob --- Parcel::Blob::Blob() : From cbf3726357966539c2a685f46e61c3fc8937f19e Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Thu, 22 Oct 2015 16:12:53 -0700 Subject: [PATCH 12/14] Revert "Revert "Track ashmem memory usage in Parcel"" This reverts commit 6880307e8e35a6c484942443fb4ddd6173126152. Bug: 25004154 Change-Id: I9b432d1ebc39f3bbcd7afdefc403f0fb6ced8158 --- include/binder/Parcel.h | 6 ++++-- libs/binder/Parcel.cpp | 38 ++++++++++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/include/binder/Parcel.h b/include/binder/Parcel.h index 3ada1e909..220a9353f 100644 --- a/include/binder/Parcel.h +++ b/include/binder/Parcel.h @@ -342,9 +342,11 @@ public: private: size_t mBlobAshmemSize; + size_t mOpenAshmemSize; public: size_t getBlobAshmemSize() const; + size_t getOpenAshmemSize() const; }; // --------------------------------------------------------------------------- @@ -412,9 +414,9 @@ inline TextOutput& operator<<(TextOutput& to, const Parcel& parcel) // Generic acquire and release of objects. void acquire_object(const sp& proc, - const flat_binder_object& obj, const void* who); + const flat_binder_object& obj, const void* who, size_t* outAshmemSize); void release_object(const sp& proc, - const flat_binder_object& obj, const void* who); + const flat_binder_object& obj, const void* who, size_t* outAshmemSize); void flatten_binder(const sp& proc, const sp& binder, flat_binder_object* out); diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 7a4ddc43a..1c0358518 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -96,7 +96,7 @@ enum { }; void acquire_object(const sp& proc, - const flat_binder_object& obj, const void* who) + const flat_binder_object& obj, const void* who, size_t* outAshmemSize) { switch (obj.type) { case BINDER_TYPE_BINDER: @@ -123,8 +123,13 @@ void acquire_object(const sp& proc, return; } case BINDER_TYPE_FD: { - // intentionally blank -- nothing to do to acquire this, but we do - // recognize it as a legitimate object type. + if (obj.cookie != 0) { + // If we own an ashmem fd, keep track of how much memory it refers to. + int size = ashmem_get_size_region(obj.handle); + if (size > 0) { + *outAshmemSize += size; + } + } return; } } @@ -133,7 +138,7 @@ void acquire_object(const sp& proc, } void release_object(const sp& proc, - const flat_binder_object& obj, const void* who) + const flat_binder_object& obj, const void* who, size_t* outAshmemSize) { switch (obj.type) { case BINDER_TYPE_BINDER: @@ -160,7 +165,14 @@ void release_object(const sp& proc, return; } case BINDER_TYPE_FD: { - if (obj.cookie != 0) close(obj.handle); + if (obj.cookie != 0) { + int size = ashmem_get_size_region(obj.handle); + if (size > 0) { + *outAshmemSize -= size; + } + + close(obj.handle); + } return; } } @@ -504,7 +516,7 @@ status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) flat_binder_object* flat = reinterpret_cast(mData + off); - acquire_object(proc, *flat, this); + acquire_object(proc, *flat, this, &mOpenAshmemSize); if (flat->type == BINDER_TYPE_FD) { // If this is a file descriptor, we need to dup it so the @@ -1026,7 +1038,7 @@ restart_write: // Need to write meta-data? if (nullMetaData || val.binder != 0) { mObjects[mObjectsSize] = mDataPos; - acquire_object(ProcessState::self(), val, this); + acquire_object(ProcessState::self(), val, this, &mOpenAshmemSize); mObjectsSize++; } @@ -1609,7 +1621,7 @@ void Parcel::releaseObjects() i--; const flat_binder_object* flat = reinterpret_cast(data+objects[i]); - release_object(proc, *flat, this); + release_object(proc, *flat, this, &mOpenAshmemSize); } } @@ -1623,7 +1635,7 @@ void Parcel::acquireObjects() i--; const flat_binder_object* flat = reinterpret_cast(data+objects[i]); - acquire_object(proc, *flat, this); + acquire_object(proc, *flat, this, &mOpenAshmemSize); } } @@ -1805,7 +1817,7 @@ status_t Parcel::continueWrite(size_t desired) // will need to rescan because we may have lopped off the only FDs mFdsKnown = false; } - release_object(proc, *flat, this); + release_object(proc, *flat, this, &mOpenAshmemSize); } binder_size_t* objects = (binder_size_t*)realloc(mObjects, objectsSize*sizeof(binder_size_t)); @@ -1891,6 +1903,7 @@ void Parcel::initState() mAllowFds = true; mOwner = NULL; mBlobAshmemSize = 0; + mOpenAshmemSize = 0; } void Parcel::scanForFds() const @@ -1913,6 +1926,11 @@ size_t Parcel::getBlobAshmemSize() const return mBlobAshmemSize; } +size_t Parcel::getOpenAshmemSize() const +{ + return mOpenAshmemSize; +} + // --- Parcel::Blob --- Parcel::Blob::Blob() : From 6bb3114246f6f6aa406e65452dbaa12b135029ea Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Thu, 22 Oct 2015 16:46:12 -0700 Subject: [PATCH 13/14] Maintain Parcel ABI Makes sure we don't change the memory layout of the Parcel class to maintain binary compatibility with prebuilts linking against libbinder. Bug: 25004154 Change-Id: I656687497f08bb85cefda796aafa2341e601e30a --- include/binder/Parcel.h | 6 +++--- libs/binder/Parcel.cpp | 46 +++++++++++++++++++++++++++-------------- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/include/binder/Parcel.h b/include/binder/Parcel.h index 220a9353f..16cd6cf67 100644 --- a/include/binder/Parcel.h +++ b/include/binder/Parcel.h @@ -341,10 +341,10 @@ public: }; private: - size_t mBlobAshmemSize; size_t mOpenAshmemSize; public: + // TODO: Remove once ABI can be changed. size_t getBlobAshmemSize() const; size_t getOpenAshmemSize() const; }; @@ -414,9 +414,9 @@ inline TextOutput& operator<<(TextOutput& to, const Parcel& parcel) // Generic acquire and release of objects. void acquire_object(const sp& proc, - const flat_binder_object& obj, const void* who, size_t* outAshmemSize); + const flat_binder_object& obj, const void* who); void release_object(const sp& proc, - const flat_binder_object& obj, const void* who, size_t* outAshmemSize); + const flat_binder_object& obj, const void* who); void flatten_binder(const sp& proc, const sp& binder, flat_binder_object* out); diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 1c0358518..22d7ef36c 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -124,10 +124,12 @@ void acquire_object(const sp& proc, } case BINDER_TYPE_FD: { if (obj.cookie != 0) { - // If we own an ashmem fd, keep track of how much memory it refers to. - int size = ashmem_get_size_region(obj.handle); - if (size > 0) { - *outAshmemSize += size; + if (outAshmemSize != NULL) { + // If we own an ashmem fd, keep track of how much memory it refers to. + int size = ashmem_get_size_region(obj.handle); + if (size > 0) { + *outAshmemSize += size; + } } } return; @@ -137,7 +139,13 @@ void acquire_object(const sp& proc, ALOGD("Invalid object type 0x%08x", obj.type); } -void release_object(const sp& proc, +void acquire_object(const sp& proc, + const flat_binder_object& obj, const void* who) +{ + acquire_object(proc, obj, who, NULL); +} + +static void release_object(const sp& proc, const flat_binder_object& obj, const void* who, size_t* outAshmemSize) { switch (obj.type) { @@ -165,13 +173,15 @@ void release_object(const sp& proc, return; } case BINDER_TYPE_FD: { - if (obj.cookie != 0) { - int size = ashmem_get_size_region(obj.handle); - if (size > 0) { - *outAshmemSize -= size; - } + if (outAshmemSize != NULL) { + if (obj.cookie != 0) { + int size = ashmem_get_size_region(obj.handle); + if (size > 0) { + *outAshmemSize -= size; + } - close(obj.handle); + close(obj.handle); + } } return; } @@ -180,6 +190,12 @@ void release_object(const sp& proc, ALOGE("Invalid object type 0x%08x", obj.type); } +void release_object(const sp& proc, + const flat_binder_object& obj, const void* who) +{ + release_object(proc, obj, who, NULL); +} + inline static status_t finish_flatten_binder( const sp& /*binder*/, const flat_binder_object& flat, Parcel* out) { @@ -935,8 +951,6 @@ status_t Parcel::writeBlob(size_t len, bool mutableCopy, WritableBlob* outBlob) int fd = ashmem_create_region("Parcel Blob", len); if (fd < 0) return NO_MEMORY; - mBlobAshmemSize += len; - int result = ashmem_set_prot_region(fd, PROT_READ | PROT_WRITE); if (result < 0) { status = result; @@ -1902,7 +1916,6 @@ void Parcel::initState() mFdsKnown = true; mAllowFds = true; mOwner = NULL; - mBlobAshmemSize = 0; mOpenAshmemSize = 0; } @@ -1923,7 +1936,10 @@ void Parcel::scanForFds() const size_t Parcel::getBlobAshmemSize() const { - return mBlobAshmemSize; + // This used to return the size of all blobs that were written to ashmem, now we're returning + // the ashmem currently referenced by this Parcel, which should be equivalent. + // TODO: Remove method once ABI can be changed. + return mOpenAshmemSize; } size_t Parcel::getOpenAshmemSize() const From 20483c49377fdb0330d9dfbbb2168b470c0b29d3 Mon Sep 17 00:00:00 2001 From: Peng Xu Date: Mon, 26 Oct 2015 15:14:43 -0700 Subject: [PATCH 14/14] Avoiding flush on-change sensors at subscription Initial sensor flush at subscription is a mechanism to avoid sensors to get stale samples before subscription happens. However, there is a slight chance that a most recent sample will be lost during the flush process. This is OK for continuous sensors but problematic in on-change sensor as on-change event does not come continuously and a lost event can cause inconsistent state in client. Flush at subscription of on-change sensor is disabled in this CL to avoid new important on-change event to be discarded during the initial flush process. Bugs: b/24647069 b/25241873 b/24804819 Change-Id: Ibda099c6b9f5fb6e200f13cf13a850b0026e9e7c --- services/sensorservice/SensorService.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp index 40b21a96a..fd72b2312 100644 --- a/services/sensorservice/SensorService.cpp +++ b/services/sensorservice/SensorService.cpp @@ -912,10 +912,15 @@ status_t SensorService::enable(const sp& connection, status_t err = sensor->batch(connection.get(), handle, 0, samplingPeriodNs, maxBatchReportLatencyNs); - // Call flush() before calling activate() on the sensor. Wait for a first flush complete - // event before sending events on this connection. Ignore one-shot sensors which don't - // support flush(). Also if this sensor isn't already active, don't call flush(). - if (err == NO_ERROR && sensor->getSensor().getReportingMode() != AREPORTING_MODE_ONE_SHOT && + // Call flush() before calling activate() on the sensor. Wait for a first + // flush complete event before sending events on this connection. Ignore + // one-shot sensors which don't support flush(). Ignore on-change sensors + // to maintain the on-change logic (any on-change events except the initial + // one should be trigger by a change in value). Also if this sensor isn't + // already active, don't call flush(). + if (err == NO_ERROR && + sensor->getSensor().getReportingMode() != AREPORTING_MODE_ONE_SHOT && + sensor->getSensor().getReportingMode() != AREPORTING_MODE_ON_CHANGE && rec->getNumConnections() > 1) { connection->setFirstFlushPending(handle, true); status_t err_flush = sensor->flush(connection.get(), handle);