From a4c0db7c61bccc6b67caddc23bd3a00dfaafa20c Mon Sep 17 00:00:00 2001 From: Timothy Yung Date: Tue, 5 Jul 2016 13:15:58 -0700 Subject: [PATCH] Revise ResponderTouchHistoryStore Error Handling (#7143) Touch behavior is inconsistent across different platforms, and ResponderTouchHistoryStore currently fatals when assumptions are broken. In addition, the behavior differs between development and production. This pull request does a few things to make ResponderTouchHistoryStore easier to deal with: Adds Flow to keep the TouchEvent, Touch, and TouchRecord types straight. Changes behavior to be consistent across environments. This means either always throwing or never throwing (and making use of warning and console.error as appropriate). When an orphaned move or end event is received, print debug information and ignore it instead of crashing and burning. --- .../eventPlugins/ResponderEventPlugin.js | 2 +- .../ResponderTouchHistoryStore.js | 265 ++++++++++-------- 2 files changed, 155 insertions(+), 112 deletions(-) diff --git a/src/renderers/shared/stack/event/eventPlugins/ResponderEventPlugin.js b/src/renderers/shared/stack/event/eventPlugins/ResponderEventPlugin.js index 564d46ef78552..33cc332bf2e32 100644 --- a/src/renderers/shared/stack/event/eventPlugins/ResponderEventPlugin.js +++ b/src/renderers/shared/stack/event/eventPlugins/ResponderEventPlugin.js @@ -496,7 +496,7 @@ var ResponderEventPlugin = { ); } - ResponderTouchHistoryStore.recordTouchTrack(topLevelType, nativeEvent, nativeEventTarget); + ResponderTouchHistoryStore.recordTouchTrack(topLevelType, nativeEvent); var extracted = canTriggerTransfer(topLevelType, targetInst, nativeEvent) ? setResponderAndExtractTransfer( diff --git a/src/renderers/shared/stack/event/eventPlugins/ResponderTouchHistoryStore.js b/src/renderers/shared/stack/event/eventPlugins/ResponderTouchHistoryStore.js index 128494333d001..94012a65524e5 100644 --- a/src/renderers/shared/stack/event/eventPlugins/ResponderTouchHistoryStore.js +++ b/src/renderers/shared/stack/event/eventPlugins/ResponderTouchHistoryStore.js @@ -7,37 +7,44 @@ * of patent rights can be found in the PATENTS file in the same directory. * * @providesModule ResponderTouchHistoryStore + * @flow */ 'use strict'; -var EventPluginUtils = require('EventPluginUtils'); +const EventPluginUtils = require('EventPluginUtils'); -var invariant = require('invariant'); +const invariant = require('invariant'); +const warning = require('warning'); -var isMoveish = EventPluginUtils.isMoveish; -var isStartish = EventPluginUtils.isStartish; -var isEndish = EventPluginUtils.isEndish; - -var MAX_TOUCH_BANK = 20; +const { + isEndish, + isMoveish, + isStartish, +} = EventPluginUtils; /** - * Touch position/time tracking information by touchID. Typically, we'll only - * see IDs with a range of 1-20 (they are recycled when touches end and then - * start again). This data is commonly needed by many different interaction - * logic modules so precomputing it is very helpful to do once. - * Each touch object in `touchBank` is of the following form: - * { touchActive: boolean, - * startTimeStamp: number, - * startPageX: number, - * startPageY: number, - * currentPageX: number, - * currentPageY: number, - * currentTimeStamp: number - * } + * Tracks the position and time of each active touch by `touch.identifier`. We + * should typically only see IDs in the range of 1-20 because IDs get recycled + * when touches end and start again. */ -var touchHistory = { - touchBank: [], +type TouchRecord = { + touchActive: boolean, + startPageX: number, + startPageY: number, + startTimeStamp: number, + currentPageX: number, + currentPageY: number, + currentTimeStamp: number, + previousPageX: number, + previousPageY: number, + previousTimeStamp: number, +}; + +const MAX_TOUCH_BANK = 20; +const touchBank: Array = []; +const touchHistory = { + touchBank, numberActiveTouches: 0, // If there is only one active touch, we remember its location. This prevents // us having to loop through all of the touches all the time in the most @@ -46,24 +53,34 @@ var touchHistory = { mostRecentTimeStamp: 0, }; -var timestampForTouch = function(touch) { +type Touch = { + identifier: ?number, + pageX: number, + pageY: number, + timestamp: number, +}; +type TouchEvent = { + changedTouches: Array, + touches: Array, +}; + +function timestampForTouch(touch: Touch): number { // The legacy internal implementation provides "timeStamp", which has been // renamed to "timestamp". Let both work for now while we iron it out // TODO (evv): rename timeStamp to timestamp in internal code - return touch.timeStamp || touch.timestamp; -}; + return (touch: any).timeStamp || touch.timestamp; +} /** * TODO: Instead of making gestures recompute filtered velocity, we could * include a built in velocity computation that can be reused globally. - * @param {Touch} touch Native touch object. */ -var initializeTouchData = function(touch) { +function createTouchRecord(touch: Touch): TouchRecord { return { touchActive: true, - startTimeStamp: timestampForTouch(touch), startPageX: touch.pageX, startPageY: touch.pageY, + startTimeStamp: timestampForTouch(touch), currentPageX: touch.pageX, currentPageY: touch.pageY, currentTimeStamp: timestampForTouch(touch), @@ -71,114 +88,140 @@ var initializeTouchData = function(touch) { previousPageY: touch.pageY, previousTimeStamp: timestampForTouch(touch), }; -}; - -var reinitializeTouchTrack = function(touchTrack, touch) { - touchTrack.touchActive = true; - touchTrack.startTimeStamp = timestampForTouch(touch); - touchTrack.startPageX = touch.pageX; - touchTrack.startPageY = touch.pageY; - touchTrack.currentPageX = touch.pageX; - touchTrack.currentPageY = touch.pageY; - touchTrack.currentTimeStamp = timestampForTouch(touch); - touchTrack.previousPageX = touch.pageX; - touchTrack.previousPageY = touch.pageY; - touchTrack.previousTimeStamp = timestampForTouch(touch); -}; - -var validateTouch = function(touch) { - var identifier = touch.identifier; - invariant(identifier != null, 'Touch object is missing identifier'); - if (identifier > MAX_TOUCH_BANK) { - console.warn( - 'Touch identifier ' + identifier + ' is greater than maximum ' + - 'supported ' + MAX_TOUCH_BANK + ' which causes performance issues ' + - 'backfilling array locations for all of the indices.' - ); - } -}; - -var recordStartTouchData = function(touch) { - var touchBank = touchHistory.touchBank; - var identifier = touch.identifier; - var touchTrack = touchBank[identifier]; - if (__DEV__) { - validateTouch(touch); - } - if (touchTrack) { - reinitializeTouchTrack(touchTrack, touch); +} + +function resetTouchRecord(touchRecord: TouchRecord, touch: Touch): void { + touchRecord.touchActive = true; + touchRecord.startPageX = touch.pageX; + touchRecord.startPageY = touch.pageY; + touchRecord.startTimeStamp = timestampForTouch(touch); + touchRecord.currentPageX = touch.pageX; + touchRecord.currentPageY = touch.pageY; + touchRecord.currentTimeStamp = timestampForTouch(touch); + touchRecord.previousPageX = touch.pageX; + touchRecord.previousPageY = touch.pageY; + touchRecord.previousTimeStamp = timestampForTouch(touch); +} + +function getTouchIdentifier({identifier}: Touch): number { + invariant(identifier != null, 'Touch object is missing identifier.'); + warning( + identifier <= MAX_TOUCH_BANK, + 'Touch identifier %s is greater than maximum supported %s which causes ' + + 'performance issues backfilling array locations for all of the indices.', + identifier, + MAX_TOUCH_BANK + ); + return identifier; +} + +function recordTouchStart(touch: Touch): void { + const identifier = getTouchIdentifier(touch); + const touchRecord = touchBank[identifier]; + if (touchRecord) { + resetTouchRecord(touchRecord, touch); } else { - touchBank[touch.identifier] = initializeTouchData(touch); + touchBank[identifier] = createTouchRecord(touch); } touchHistory.mostRecentTimeStamp = timestampForTouch(touch); -}; - -var recordMoveTouchData = function(touch) { - var touchBank = touchHistory.touchBank; - var touchTrack = touchBank[touch.identifier]; - if (__DEV__) { - validateTouch(touch); - invariant(touchTrack, 'Touch data should have been recorded on start'); +} + +function recordTouchMove(touch: Touch): void { + const touchRecord = touchBank[getTouchIdentifier(touch)]; + if (touchRecord) { + touchRecord.touchActive = true; + touchRecord.previousPageX = touchRecord.currentPageX; + touchRecord.previousPageY = touchRecord.currentPageY; + touchRecord.previousTimeStamp = touchRecord.currentTimeStamp; + touchRecord.currentPageX = touch.pageX; + touchRecord.currentPageY = touch.pageY; + touchRecord.currentTimeStamp = timestampForTouch(touch); + touchHistory.mostRecentTimeStamp = timestampForTouch(touch); + } else { + console.error( + 'Cannot record touch move without a touch start.\n' + + 'Touch Move: %s\n', + 'Touch Bank: %s', + printTouch(touch), + printTouchBank() + ); } - touchTrack.touchActive = true; - touchTrack.previousPageX = touchTrack.currentPageX; - touchTrack.previousPageY = touchTrack.currentPageY; - touchTrack.previousTimeStamp = touchTrack.currentTimeStamp; - touchTrack.currentPageX = touch.pageX; - touchTrack.currentPageY = touch.pageY; - touchTrack.currentTimeStamp = timestampForTouch(touch); - touchHistory.mostRecentTimeStamp = timestampForTouch(touch); -}; - -var recordEndTouchData = function(touch) { - var touchBank = touchHistory.touchBank; - var touchTrack = touchBank[touch.identifier]; - if (__DEV__) { - validateTouch(touch); - invariant(touchTrack, 'Touch data should have been recorded on start'); +} + +function recordTouchEnd(touch: Touch): void { + const touchRecord = touchBank[getTouchIdentifier(touch)]; + if (touchRecord) { + touchRecord.touchActive = false; + touchRecord.previousPageX = touchRecord.currentPageX; + touchRecord.previousPageY = touchRecord.currentPageY; + touchRecord.previousTimeStamp = touchRecord.currentTimeStamp; + touchRecord.currentPageX = touch.pageX; + touchRecord.currentPageY = touch.pageY; + touchRecord.currentTimeStamp = timestampForTouch(touch); + touchHistory.mostRecentTimeStamp = timestampForTouch(touch); + } else { + console.error( + 'Cannot record touch end without a touch start.\n' + + 'Touch End: %s\n', + 'Touch Bank: %s', + printTouch(touch), + printTouchBank() + ); } - touchTrack.previousPageX = touchTrack.currentPageX; - touchTrack.previousPageY = touchTrack.currentPageY; - touchTrack.previousTimeStamp = touchTrack.currentTimeStamp; - touchTrack.currentPageX = touch.pageX; - touchTrack.currentPageY = touch.pageY; - touchTrack.currentTimeStamp = timestampForTouch(touch); - touchTrack.touchActive = false; - touchHistory.mostRecentTimeStamp = timestampForTouch(touch); -}; +} + +function printTouch(touch: Touch): string { + return JSON.stringify({ + identifier: touch.identifier, + pageX: touch.pageX, + pageY: touch.pageY, + timestamp: timestampForTouch(touch), + }); +} + +function printTouchBank(): string { + let printed = JSON.stringify(touchBank.slice(0, MAX_TOUCH_BANK)); + if (touchBank.length > MAX_TOUCH_BANK) { + printed += ' (original size: ' + touchBank.length + ')'; + } + return printed; +} -var ResponderTouchHistoryStore = { - recordTouchTrack: function(topLevelType, nativeEvent) { - var touchBank = touchHistory.touchBank; +const ResponderTouchHistoryStore = { + recordTouchTrack(topLevelType: string, nativeEvent: TouchEvent): void { if (isMoveish(topLevelType)) { - nativeEvent.changedTouches.forEach(recordMoveTouchData); + nativeEvent.changedTouches.forEach(recordTouchMove); } else if (isStartish(topLevelType)) { - nativeEvent.changedTouches.forEach(recordStartTouchData); + nativeEvent.changedTouches.forEach(recordTouchStart); touchHistory.numberActiveTouches = nativeEvent.touches.length; if (touchHistory.numberActiveTouches === 1) { - touchHistory.indexOfSingleActiveTouch = nativeEvent.touches[0].identifier; + touchHistory.indexOfSingleActiveTouch = + nativeEvent.touches[0].identifier; } } else if (isEndish(topLevelType)) { - nativeEvent.changedTouches.forEach(recordEndTouchData); + nativeEvent.changedTouches.forEach(recordTouchEnd); touchHistory.numberActiveTouches = nativeEvent.touches.length; if (touchHistory.numberActiveTouches === 1) { - for (var i = 0; i < touchBank.length; i++) { - var touchTrackToCheck = touchBank[i]; + for (let i = 0; i < touchBank.length; i++) { + const touchTrackToCheck = touchBank[i]; if (touchTrackToCheck != null && touchTrackToCheck.touchActive) { touchHistory.indexOfSingleActiveTouch = i; break; } } if (__DEV__) { - var activeTouchData = touchBank[touchHistory.indexOfSingleActiveTouch]; - var foundActive = activeTouchData != null && !!activeTouchData.touchActive; - invariant(foundActive, 'Cannot find single active touch'); + const activeRecord = touchBank[touchHistory.indexOfSingleActiveTouch]; + warning( + activeRecord != null && + activeRecord.touchActive, + 'Cannot find single active touch.' + ); } } } }, - touchHistory: touchHistory, + touchHistory, };