New issue
Advanced search Search tips

Issue 646588 link

Starred by 1 user
Status: Fixed
Owner:
dmazz...@chromium.org
Closed: Sep 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Team-Accessibility



Sign in to add a comment

RendererAccessibilityImpl is crasing on DCHECK(ack_pending_)

Project Member Reported by dmazz...@chromium.org, Sep 13 2016 
I can repro this on Android WebView with accessibility enabled, as sometimes closing and opening a page with a WebView toggles accessibility quickly.

The issue is that toggling accessibility off for a RenderFrameImpl deletes the RenderAccessibilityImpl, but this can happen after the RenderAccessibilityImpl has already sent an IPC to the browser.

The browser sends an Ack, and in the meantime if the RenderFrameImpl creates a new RenderAccessibilityImpl, that RenderAccessibilityImpl is confused by the Ack because it didn't think it sent one.

As a fix, pass an ack token from the renderer that must be returned from the browser.

Found on Android but seems like it could happen on any platform.

Relatively harmless in release mode with DCHECK disabled.
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/50aa111be5288fbc06b54e09d2cafd4dd294a315

commit 50aa111be5288fbc06b54e09d2cafd4dd294a315
Author: dmazzoni <dmazzoni@chromium.org>
Date: Thu Sep 15 17:59:05 2016

Fix race condition causing DCHECK(ack_pending_) to trip.

If RenderFrameImpl deletes a RenderAccessibilityImpl and then recreates
another one very quickly, that second RenderAccessibilityImpl can get
confused when it receives an ACK intended for the previous instance.

Fix this by passing an ACK token from the renderer to browser and ignoring
messages with the wrong token.

This only caused issues when DCHECKs are enabled.

BUG= 646588 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2341503002
Cr-Commit-Position: refs/heads/master@{#418902}

[modify] https://crrev.com/50aa111be5288fbc06b54e09d2cafd4dd294a315/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/50aa111be5288fbc06b54e09d2cafd4dd294a315/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/50aa111be5288fbc06b54e09d2cafd4dd294a315/content/common/accessibility_messages.h
[modify] https://crrev.com/50aa111be5288fbc06b54e09d2cafd4dd294a315/content/renderer/accessibility/render_accessibility_impl.cc
[modify] https://crrev.com/50aa111be5288fbc06b54e09d2cafd4dd294a315/content/renderer/accessibility/render_accessibility_impl.h
[modify] https://crrev.com/50aa111be5288fbc06b54e09d2cafd4dd294a315/content/renderer/accessibility/render_accessibility_impl_browsertest.cc

Comment 2 by dmazz...@chromium.org, Sep 15 2016

Status: Fixed (was: Assigned)
►

Sign in to add a comment