general note: content layer and below should never modify any app-wide settings on android, because in android webview, content code is only part of the app. Imagine in using a random library in your app and it has undocumented side effects as well.

That CL is fixing a real memory bug, but in the wrong way..

From the CL description, sounds like better fix is to have BrowserAccessibilityManagerAndroid hold a weakptr of WebContentsAccessibilityAndroid?

Looks like the same root cause as  http://crbug.com/775532  - did https://chromium-review.googlesource.com/c/chromium/src/+/795478 not fix this?

I guess so, because I just synced and now CTS passes >.< so hooray for duplicated effort.

That we run "accessibility_->UpdateEnabledState(false);" at non-deterministic times is still worrying; do we actually need that?

If content is not allowed to set the accessibility mode, we're surely breaking that rule in more places than ~Connector() :/ Is that something we could actually fix?

Yes, you should be able to just null the pointer.

When looking at a fix, I started in that manner but while adding several new methods to make such a thing possible from the place that new it needed to be done, I discovered that it already existed in the general disable of the ability.  Since I don't work on Accessibility, WebContents, or even Android, I went with the simplest solution I could find.

Doesn't Accessibility get re-enabled when another WebContents becomes active?

The problem, btw, wasn't that the BrowserAccessibilityManagerAndroid pointer wasn't checked to be non-NULL but rather that the object itself got destructed while that pointer remained.  This led to a use-after-free which ended in random memory corruption.  That particular bit of fun took about 3 months to track down so please be careful in whatever solution you decide.

> Doesn't Accessibility get re-enabled when another WebContents becomes active?

That would help if each WebContents were destroyed before creating a new one. But with WebView, AwContents (and therefore WebContents) are created and destroyed at arbitrary times.

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

commit ce447ed4f2dd32fdbf25995ef456e990fcc72328
Author: Paul Miller <paulmiller@google.com>
Date: Tue Dec 12 21:04:33 2017

A11y: Refactor out UpdateEnabledState

UpdateEnabledState is really 2 functions in one:
UpdateEnabledState(true) is only called from Enable, and
UpdateEnabledState(false) is only called from ~Connector. Despite its
name, UpdateEnabledState(false) must not actually disable accessibility,
because of bugs  775532  and  792269 . So fold the 2 parts into their
respective call sites.

BUG= 792269 

Change-Id: Ie7afcebda79820d4856a8b050f9b865c2bd1f5c3
Reviewed-on: https://chromium-review.googlesource.com/812084
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Paul Miller <paulmiller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523540}
[modify] https://crrev.com/ce447ed4f2dd32fdbf25995ef456e990fcc72328/content/browser/accessibility/web_contents_accessibility_android.cc
[modify] https://crrev.com/ce447ed4f2dd32fdbf25995ef456e990fcc72328/content/browser/accessibility/web_contents_accessibility_android.h