Last Comment Bug 741666 - touches for touchLists are not refcounted correctly
: touches for touchLists are not refcounted correctly
Product: Core
Classification: Components
Component: Event Handling (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla14
Assigned To: Wesley Johnston (:wesj)
: Andrew Overholt [:overholt]
Depends on: 964153
  Show dependency treegraph
Reported: 2012-04-02 18:29 PDT by Wesley Johnston (:wesj)
Modified: 2014-01-26 22:23 PST (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (7.35 KB, patch)
2012-04-03 17:24 PDT, Wesley Johnston (:wesj)
bugs: review+
Details | Diff | Splinter Review
Patch v2 (9.13 KB, patch)
2012-04-03 19:42 PDT, Wesley Johnston (:wesj)
bugs: review+
Details | Diff | Splinter Review

Description User image Wesley Johnston (:wesj) 2012-04-02 18:29:09 PDT
If I do the following:

addEventListener("touchstart", function(aEvent) {
  // console.log(aEvent.touches.length); // If I include this line,
                                         // the second log will return 1, otherwise 0
  setTimeout(function() {
  }, 0);
}, false);

I find that all the touch lists are length 0 inside the setTimeout loop. We lazily build and cache the touchlist when you access it. If we don't create the cache outside the loop, the nsTArray of touches held by our nsTouchEvent is being destroyed, and we build a touch list of length 0.
Comment 1 User image Wesley Johnston (:wesj) 2012-04-03 17:24:39 PDT
Created attachment 612034 [details] [diff] [review]

We aren't duplicating these correctly.

Fix + test. Still building to run the test here, and will push to try as well.
Comment 2 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-04-03 17:32:25 PDT
Comment on attachment 612034 [details] [diff] [review]

>     case NS_TOUCH_EVENT:
>     {
>-      newEvent = new nsTouchEvent(false, msg, nsnull);
>+      nsTouchEvent *oldTouchEvent = static_cast<nsTouchEvent*>(mEvent);
>+      newEvent = new nsTouchEvent(oldTouchEvent);
>       isInputEvent = true;
>       break;
To be consistent with other events, make sure newEvent is not trusted, and add a test for that.
(your code makes it now trusted, if oldTouchEvent is trusted)

with that, r=me
Comment 3 User image Wesley Johnston (:wesj) 2012-04-03 19:42:41 PDT
Created attachment 612073 [details] [diff] [review]
Patch v2

This modifies our constructor so that it takes an isTruested parameter and passes in false like everyone else, but in (all?) cases, that gets overridden down here:

For now I left that in (but the test checks that isTrusted is always true now). Do we want to change that behavior?
Comment 4 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-04-03 19:46:40 PDT
Comment on attachment 612073 [details] [diff] [review]
Patch v2

>-      nsTouchEvent newEvent(touchEvent);
>+      nsTouchEvent newEvent(touchEvent->flags & NS_EVENT_FLAG_TRUSTED ?
>+                              true : false,

Note You need to log in before you can comment on or make changes to this bug.