Issue metadata
Sign in to add a comment
|
AccessibilityEventRecorderWin issues |
||||||||||||||||||||||||
Issue description
When I'm doing a new feature, currently I try to create/destroy a OS session monitor in content/browser/browser_main_loop, in which a base::win::MessageWindow will be created/destroyed in the monitor at ctor/dtor of browser_main_loop. And this attempt will trigger DumpAccessibilityEventsTest error on Win10 bots, as the destruct of MessageWindow will cause a Windows event which will be caught by AccessibilityEventRecorderWin.
Then I notice that even without my codes, the ToT chromium also has problem with the DumpAccessibilityEventsTest too.
The attached files show the call stack of the failure of DumpAccessibilityEventsTest.AccessibilityEventsRemoveChild, with ToT chromium #590511, in great chance.
Beside these callstacks, here is some other finding:
If I add check to the "manager_->delegate()" before using it, like
if (!manager_->delegate()) {
LOG(ERROR) << __func__ << __LINE__;
return E_FAIL;
}
HWND accessibility_hwnd =
manager_->delegate()->AccessibilityGetAcceleratedWidget();
Then sometimes there is the log without crash, sometimes there is still the crash at accessing delegate()->AccessibilityGetAcceleratedWidget(), which implies that this should be a race condition issue. (e.g. try more times to see the reproduction :) )
,
Sep 18
Any update?
,
Sep 19
The problem seems that the event recorder on Win/Linux/OSX is created as singleton, which means the dtor (in which it supposes to stop listening) of the recorder won't be called at all. So if any event is caught during destructing, then race condition may happen around accessibility_event_recorder_win.cc:360 on Win (maybe same on other platforms too.). It seems that the AccessibilityEventRecoder is only for unittests. Is that true?
,
Sep 19
just noticed that this is newly introduced in https://chromium-review.googlesource.com/c/chromium/src/+/1171562. nektar@, please take a look. The change will cause accessibility unittests randomly crash on Win after it reports test success(see the call stack in original report). And this also blocks my ongoing work. Maybe we should go back to the original Create() way first?
,
Sep 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5715b273dfcb552f461aef46e14b2f466d5412b5 commit 5715b273dfcb552f461aef46e14b2f466d5412b5 Author: braveyao <braveyao@chromium.org> Date: Thu Sep 20 15:50:35 2018 Restore Create() method instead of singleton of AcceessibillityRecorder In a recent cl, https://chromium-review.googlesource.com/c/chromium/src/+/1171562, it starts to use singleton to create AcceessibillityRecorder instance. This causes a problem in crbug.com/883136 , that because the dtor will never be called, windowns event will still be caught and processed during destuction period, which may randomly crash due to race condition. This cl is to partially revert the changes to stop listening correctly at destruction. Bug: 883136 Change-Id: I1bd8d3e00b23590c222652c15632e33b4e3667e9 Reviewed-on: https://chromium-review.googlesource.com/1234853 Commit-Queue: Nektarios Paisios <nektar@chromium.org> Reviewed-by: Nektarios Paisios <nektar@chromium.org> Cr-Commit-Position: refs/heads/master@{#592815} [modify] https://crrev.com/5715b273dfcb552f461aef46e14b2f466d5412b5/content/browser/accessibility/accessibility_event_recorder.cc [modify] https://crrev.com/5715b273dfcb552f461aef46e14b2f466d5412b5/content/browser/accessibility/accessibility_event_recorder.h [modify] https://crrev.com/5715b273dfcb552f461aef46e14b2f466d5412b5/content/browser/accessibility/accessibility_event_recorder_auralinux.cc [modify] https://crrev.com/5715b273dfcb552f461aef46e14b2f466d5412b5/content/browser/accessibility/accessibility_event_recorder_mac.mm [modify] https://crrev.com/5715b273dfcb552f461aef46e14b2f466d5412b5/content/browser/accessibility/accessibility_event_recorder_win.cc [modify] https://crrev.com/5715b273dfcb552f461aef46e14b2f466d5412b5/content/browser/accessibility/accessibility_win_browsertest.cc [modify] https://crrev.com/5715b273dfcb552f461aef46e14b2f466d5412b5/content/browser/accessibility/dump_accessibility_events_browsertest.cc [modify] https://crrev.com/5715b273dfcb552f461aef46e14b2f466d5412b5/tools/accessibility/inspect/ax_event_server.cc [modify] https://crrev.com/5715b273dfcb552f461aef46e14b2f466d5412b5/tools/accessibility/inspect/ax_event_server.h
,
Sep 20
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by braveyao@chromium.org
, Sep 11Owner: dmazz...@chromium.org
Status: Assigned (was: Untriaged)