Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(33)

Issue 869433007: Revert of Don't refcount tracking id -> slot id mapping. (Closed)

Created:
5 years, 10 months ago by tdresser
Modified:
5 years, 10 months ago
Reviewers:
sadrul
CC:
chromium-reviews, kalyank, tfarina, jdduke+watch_chromium.org, ozone-reviews_chromium.org, tdresser+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Don't refcount tracking id -> slot id mapping. (patchset #6 id:120001 of https://codereview.chromium.org/785753002/) Reason for revert: Reverting due to memory failures. http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%283%29/builds/5041/steps/memory%20test%3A%20content/logs/stdio { UNINITIALIZED READ name=<insert_a_suppression_name_here> content.dll!content::TouchEventQueue::TouchMoveSlopSuppressor::FilterEvent content.dll!content::TouchEventQueue::FilterBeforeForwarding content.dll!content::TouchEventQueue::TryForwardNextEventToRenderer content.dll!content::TouchEventQueue::ProcessTouchAck content.dll!content::InputRouterImpl::ProcessInputEventAck content.dll!content::InputRouterImpl::OnInputEventAck content.dll!InputHostMsg_HandleInputEvent_ACK::Dispatch<> content.dll!content::InputRouterImpl::OnMessageReceived content.dll!content::RenderWidgetHostImpl::OnMessageReceived *!content::RenderWidgetHostViewAuraTest_TouchEventState_Test::TestBody *!testing::internal::HandleExceptionsInMethodIfSupported<> } http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28valgrind%29%284%29/builds/40143/steps/memory%20test%3A%20content/logs/stdio UninitCondition Conditional jump or move depends on uninitialised value(s) content::TouchEventQueue::TouchMoveSlopSuppressor::FilterEvent(blink::WebTouchEvent const&) (content/browser/renderer_host/input/touch_event_queue.cc:240) content::TouchEventQueue::FilterBeforeForwarding(blink::WebTouchEvent const&) (content/browser/renderer_host/input/touch_event_queue.cc:715) content::TouchEventQueue::TryForwardNextEventToRenderer() (content/browser/renderer_host/input/touch_event_queue.cc:445) content::TouchEventQueue::ProcessTouchAck(content::InputEventAckState, ui::LatencyInfo const&) (content/browser/renderer_host/input/touch_event_queue.cc:436) content::InputRouterImpl::ProcessTouchAck(content::InputEventAckState, ui::LatencyInfo const&) (content/browser/renderer_host/input/input_router_impl.cc:666) content::InputRouterImpl::ProcessInputEventAck(blink::WebInputEvent::Type, content::InputEventAckState, ui::LatencyInfo const&, content::InputRouterImpl::AckSource) (content/browser/renderer_host/input/input_router_impl.cc:576) content::InputRouterImpl::OnInputEventAck(InputHostMsg_HandleInputEvent_ACK_Params const&) (content/browser/renderer_host/input/input_router_impl.cc:483) _Z20DispatchToMethodImplIN7content15InputRouterImplEMS1_FvRK40InputHostMsg_HandleInputEvent_ACK_ParamsEJS2_EJLm0EEEvPT_T0_RK5TupleIJDpT1_EE13IndexSequenceIJXspT2_EEE (base/tuple.h:246) Suppression (error hash=#675E7177BFE2BE12#): For more info on using suppressions see http://dev.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/memory-sheriff#TOC-Suppressing-memory-reports { <insert_a_suppression_name_here> Memcheck:Uninitialized fun:_ZN7content15TouchEventQueue23TouchMoveSlopSuppressor11FilterEventERKN5blink13WebTouchEventE fun:_ZN7content15TouchEventQueue22FilterBeforeForwardingERKN5blink13WebTouchEventE fun:_ZN7content15TouchEventQueue29TryForwardNextEventToRendererEv fun:_ZN7content15TouchEventQueue15ProcessTouchAckENS_18InputEventAckStateERKN2ui11LatencyInfoE fun:_ZN7content15InputRouterImpl15ProcessTouchAckENS_18InputEventAckStateERKN2ui11LatencyInfoE fun:_ZN7content15InputRouterImpl20ProcessInputEventAckEN5blink13WebInputEvent4TypeENS_18InputEventAckStateERKN2ui11LatencyInfoENS0_9AckSourceE fun:_ZN7content15InputRouterImpl15OnInputEventAckERK40InputHostMsg_HandleInputEvent_ACK_Params fun:_Z20DispatchToMethodImplIN7content15InputRouterImplEMS1_FvRK40InputHostMsg_HandleInputEvent_ACK_ParamsEJS2_EJLm0EEEvPT_T0_RK5TupleIJDpT1_EE13IndexSequenceIJXspT2_EEE } I haven't been able to reproduce this consistently locally, which is making it tricky to pin down. Once I have (or I've determined this patch isn't what's causing the failures), I'll reland. Original issue's description: > Don't refcount tracking id -> slot id mapping. > > Previously we tried to refcount the tracking id to slot id mapping. > This broke in some circumstances where the number of press events was > not equal to the number of release events. > > This patch switches to marking some touch events such that they don't > modify the mapping, simplifying logic, and fixing a nasty bug. > > BUG=439051 > TEST=EventsXTest.TouchEventNotRemovingFromNativeMapping > > Committed: https://crrev.com/47a823b565a4051e58d83a857738c0fb8417d9ac > Cr-Commit-Position: refs/heads/master@{#313520} TBR=sadrul@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=439051 Committed: https://crrev.com/3513587cce08b0b7ac35f16d5ca1dd8d30b02708 Cr-Commit-Position: refs/heads/master@{#313697}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+107 lines, -140 lines) Patch
M ui/events/cocoa/events_mac.mm View 1 chunk +4 lines, -0 lines 0 comments Download
M ui/events/devices/x11/touch_factory_x11.h View 2 chunks +8 lines, -0 lines 0 comments Download
M ui/events/devices/x11/touch_factory_x11.cc View 2 chunks +8 lines, -1 line 0 comments Download
M ui/events/event.h View 6 chunks +6 lines, -15 lines 0 comments Download
M ui/events/event.cc View 4 chunks +17 lines, -18 lines 0 comments Download
M ui/events/event_utils.h View 1 chunk +5 lines, -0 lines 0 comments Download
M ui/events/events_stub.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M ui/events/ozone/events_ozone.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M ui/events/win/events_win.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M ui/events/x/events_x.cc View 1 chunk +12 lines, -0 lines 0 comments Download
M ui/events/x/events_x_unittest.cc View 1 chunk +36 lines, -29 lines 0 comments Download
M ui/views/controls/menu/menu_controller_unittest.cc View 3 chunks +0 lines, -69 lines 0 comments Download
M ui/views/controls/menu/menu_event_dispatcher_linux.cc View 1 chunk +0 lines, -8 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
tdresser
Created Revert of Don't refcount tracking id -> slot id mapping.
5 years, 10 months ago (2015-01-29 13:56:29 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/869433007/1
5 years, 10 months ago (2015-01-29 13:57:20 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 10 months ago (2015-01-29 13:58:15 UTC) #3
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/3513587cce08b0b7ac35f16d5ca1dd8d30b02708 Cr-Commit-Position: refs/heads/master@{#313697}
5 years, 10 months ago (2015-01-29 13:59:06 UTC) #4
Lei Zhang
5 years, 10 months ago (2015-02-03 00:18:38 UTC) #5
Message was sent while issue was closed.
On 2015/01/29 13:56:29, tdresser wrote:
> Created Revert of Don't refcount tracking id -> slot id mapping.

Thanks for reverting!

Powered by Google App Engine
This is Rietveld 408576698