Commit 4d7ff911 authored by Peter Collingbourne's avatar Peter Collingbourne Committed by Commit Bot

accessibility: Fix CFI bad cast errors.

Running content_unittests on Android with CFI bad cast checks enabled
[1] revealed two bad cast errors in accessibility: a bad cast
from CountedBrowserAccessibility to BrowserAccessibilityAndroid
in browser_accessibility_manager_android.cc:69 and from
TestBrowserAccessibilityManager to BrowserAccessibilityManagerAndroid
in browser_accessibility_android.cc:148.

In the former case, the code from BrowserAccessibilityAndroid can
easily be inlined into the caller, so this patch does that. In the
latter case, the caller is accessing a field that is only present
on BrowserAccessibilityManagerAndroid, so the fix is to derive
TestBrowserAccessibilityManager from BrowserAccessibilityManagerAndroid
on Android platforms.

This change hopefully fixes the memory error that was detected by
asan. In a followup I plan to attempt to re-enable these tests on
Android with asan.

[1] https://build.chromium.org/p/chromium.clang/builders/ToTAndroidCFI/builds/397/steps/content_unittests%20on%20Android

Bug: 469376
Change-Id: I4560451ec276ab1057feac3ef906daf5fc045ca3
Reviewed-on: https://chromium-review.googlesource.com/780884
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518352}
parent a3bb6ffb
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "content/browser/accessibility/browser_accessibility_android.h" #include "content/browser/accessibility/browser_accessibility_android.h"
#include "content/browser/accessibility/web_contents_accessibility_android.h" #include "content/browser/accessibility/web_contents_accessibility_android.h"
#include "content/common/accessibility_messages.h" #include "content/common/accessibility_messages.h"
#include "ui/accessibility/ax_role_properties.h"
namespace content { namespace content {
...@@ -65,9 +66,7 @@ bool BrowserAccessibilityManagerAndroid::ShouldExposePasswordText() { ...@@ -65,9 +66,7 @@ bool BrowserAccessibilityManagerAndroid::ShouldExposePasswordText() {
BrowserAccessibility* BrowserAccessibilityManagerAndroid::GetFocus() { BrowserAccessibility* BrowserAccessibilityManagerAndroid::GetFocus() {
BrowserAccessibility* focus = BrowserAccessibilityManager::GetFocus(); BrowserAccessibility* focus = BrowserAccessibilityManager::GetFocus();
BrowserAccessibilityAndroid* android_focus = if (!ui::IsEditField(focus->GetRole()))
static_cast<BrowserAccessibilityAndroid*>(focus);
if (!android_focus->IsEditableText())
return GetActiveDescendant(focus); return GetActiveDescendant(focus);
return focus; return focus;
} }
......
...@@ -10,12 +10,23 @@ ...@@ -10,12 +10,23 @@
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "content/browser/accessibility/browser_accessibility.h" #include "content/browser/accessibility/browser_accessibility.h"
#include "content/browser/accessibility/browser_accessibility_manager.h" #include "content/browser/accessibility/browser_accessibility_manager.h"
#ifdef OS_ANDROID
#include "content/browser/accessibility/browser_accessibility_manager_android.h"
#endif
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace content { namespace content {
namespace { namespace {
#ifdef OS_ANDROID
class TestBrowserAccessibilityManager
: public BrowserAccessibilityManagerAndroid {
public:
TestBrowserAccessibilityManager(const ui::AXTreeUpdate& initial_tree)
: BrowserAccessibilityManagerAndroid(initial_tree, nullptr, nullptr) {}
};
#else
class TestBrowserAccessibilityManager : public BrowserAccessibilityManager { class TestBrowserAccessibilityManager : public BrowserAccessibilityManager {
public: public:
TestBrowserAccessibilityManager( TestBrowserAccessibilityManager(
...@@ -24,6 +35,7 @@ class TestBrowserAccessibilityManager : public BrowserAccessibilityManager { ...@@ -24,6 +35,7 @@ class TestBrowserAccessibilityManager : public BrowserAccessibilityManager {
nullptr, nullptr,
new BrowserAccessibilityFactory()) {} new BrowserAccessibilityFactory()) {}
}; };
#endif
} // namespace } // namespace
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment