Bug 122804 - EventPath::updateTouchLists traverses through EventPath thrice
Summary: EventPath::updateTouchLists traverses through EventPath thrice
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-14 18:46 PDT by Ryosuke Niwa
Modified: 2013-10-14 21:06 PDT (History)
11 users (show)

See Also:

Attachments
Cleanup (7.24 KB, patch)
2013-10-14 18:54 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Fixed builds (7.06 KB, patch)
2013-10-14 19:05 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Patch for landing (7.06 KB, patch)
2013-10-14 20:01 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Another build fix attempt (7.08 KB, patch)
2013-10-14 20:41 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2013-10-14 18:46:14 PDT 
EventPath::updateTouchLists goes through m_path thrice by calling updateTouchListsInEventPath.

Given that nodes tend to be scattered across multiple pages, this is a very inefficient operation.
It's much better to loop over touch lists or touch objects while the outer loop traverses through EventPath.
Comment 1 Ryosuke Niwa 2013-10-14 18:54:54 PDT 
Created attachment 214219 [details]
Cleanup
Comment 2 EFL EWS Bot 2013-10-14 18:58:26 PDT 
Comment on attachment 214219 [details]
Cleanup

Attachment 214219 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/4039023
Comment 3 EFL EWS Bot 2013-10-14 18:59:15 PDT 
Comment on attachment 214219 [details]
Cleanup

Attachment 214219 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/4039022
Comment 4 Ryosuke Niwa 2013-10-14 19:05:53 PDT 
Created attachment 214220 [details]
Fixed builds
Comment 5 EFL EWS Bot 2013-10-14 19:09:07 PDT 
Comment on attachment 214220 [details]
Fixed builds

Attachment 214220 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/4041023
Comment 6 EFL EWS Bot 2013-10-14 19:10:54 PDT 
Comment on attachment 214220 [details]
Fixed builds

Attachment 214220 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/4001046
Comment 7 Benjamin Poulain 2013-10-14 19:12:37 PDT 
Comment on attachment 214220 [details]
Fixed builds

View in context: https://bugs.webkit.org/attachment.cgi?id=214220&action=review

> Source/WebCore/dom/EventDispatcher.cpp:444
> +    const size_t touchNodeCount = touchEvent.touches()->length() + touchEvent.targetTouches()->length() + touchEvent.changedTouches()->length();

No need for the const.

I think "touchNode" is confusing. Use totalTouchesCount instead?
Comment 8 Ryosuke Niwa 2013-10-14 20:01:25 PDT 
Created attachment 214222 [details]
Patch for landing
Comment 9 Ryosuke Niwa 2013-10-14 20:01:35 PDT 
Comment on attachment 214222 [details]
Patch for landing

Wait for EWS.
Comment 10 EFL EWS Bot 2013-10-14 20:25:04 PDT 
Comment on attachment 214222 [details]
Patch for landing

Attachment 214222 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/3914093
Comment 11 EFL EWS Bot 2013-10-14 20:26:00 PDT 
Comment on attachment 214222 [details]
Patch for landing

Attachment 214222 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/4039038
Comment 12 Ryosuke Niwa 2013-10-14 20:41:40 PDT 
Created attachment 214225 [details]
Another build fix attempt
Comment 13 Ryosuke Niwa 2013-10-14 21:06:37 PDT 
Committed r157441: <http://trac.webkit.org/changeset/157441>