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

Issue 2430473003: Revert of Create AXAction and AXActionData as a way to simplify accessibility actions (Closed)

Created:
4 years, 2 months ago by Guido Urdaneta
Modified:
4 years, 2 months ago
CC:
chromium-reviews, extensions-reviews_chromium.org, sadrul, mlamouri+watch-content_chromium.org, aboxhall+watch_chromium.org, jam, creis+watch_chromium.org, nasko+codewatch_chromium.org, nektar+watch_chromium.org, yuzo+watch_chromium.org, je_julie, darin-cc_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org, kalyank, chromium-apps-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Create AXAction and AXActionData as a way to simplify accessibility actions (patchset #9 id:160001 of https://codereview.chromium.org/2410333005/ ) Reason for revert: Caused compile failure on Cast Android bot. See https://uberchromegw.corp.google.com/i/chromium.linux/builders/Cast%20Android%20%28dbg%29/builds/53236/steps/compile/logs/stdio [779/836] CXX obj/content/browser/browser/browser_accessibility_manager_android.o FAILED: obj/content/browser/browser/browser_accessibility_manager_android.o /b/c/cipd/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/content/browser/browser/browser_accessibility_manager_android.o.d -DENABLE_SCREEN_CAPTURE=1 -DAPPCACHE_USE_SIMPLE_CACHE -DUSE_MINIKIN_HYPHENATION=1 -DV8_DEPRECATION_WARNINGS -DENABLE_NOTIFICATIONS -DENABLE_PLUGINS=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DUSE_EXTERNAL_POPUP_MENU=1 -DENABLE_WEBRTC=1 -DDISABLE_NACL -DUSE_PROPRIETARY_CODECS -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=283753-1 -D_FILE_OFFSET_BITS=64 -DANDROID -DHAVE_SYS_UIO_H -DANDROID_NDK_VERSION=r12b -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__GNU_SOURCE=1 -D__compiler_offsetof=__builtin_offsetof -Dnan=__builtin_nan -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DCONTENT_IMPLEMENTATION -DV8_USE_EXTERNAL_STARTUP_DATA -DENABLE_MOJO_MEDIA -DENABLE_MOJO_CDM -DENABLE_MOJO_AUDIO_DECODER -DENABLE_MOJO_MEDIA_IN_GPU_PROCESS -DUSE_EGL -DDISABLE_FFMPEG_VIDEO_DECODERS -DLEVELDB_PLATFORM_CHROMIUM=1 -DSK_IGNORE_DW_GRAY_FIX -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_SUPPORT_GPU=1 -DSK_BUILD_FOR_ANDROID -DUSE_CHROMIUM_SKIA -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DOPUS_FIXED_POINT -DMESA_EGL_NO_X11_HEADERS -DPOSIX_AVOID_MMAP -DDISABLE_FTP_SUPPORT=1 -DENABLE_WEBSOCKETS -DFEATURE_ENABLE_SSL -DFEATURE_ENABLE_VOICEMAIL -DEXPAT_RELATIVE_PATH -DGTEST_RELATIVE_PATH -DNO_MAIN_THREAD_WRAPPING -DNO_SOUND_SYSTEM -DWEBRTC_CHROMIUM_BUILD -DWEBRTC_POSIX -DWEBRTC_LINUX -DWEBRTC_ANDROID -DXML_STATIC -DSSL_USE_OPENSSL -DHAVE_OPENSSL_SSL_H -DFEATURE_ENABLE_SSL -DLOGGING=1 -DNO_MAIN_THREAD_WRAPPING -I../.. -Igen -I../../third_party/khronos -I../../gpu -I../../third_party/leveldatabase -I../../third_party/leveldatabase/src -I../../third_party/leveldatabase/src/include -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c -I../../third_party/skia/include/config -I../../third_party/skia/include/core -I../../third_party/skia/include/effects -I../../third_party/skia/include/images -I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops -I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils -I../../third_party/skia/include/gpu -I../../third_party/skia/src/gpu -I../../third_party/skia/src/sksl -I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/protobuf/src -Igen/blink -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -I../../third_party/libwebm/source -Igen/media/base/android/media_jni_headers -Igen/media/base/android/media_jni_headers/media -I../../third_party/opus/src/include -Igen -I../../third_party/ced/src -I../../third_party/WebKit -Igen/third_party/WebKit -I../../v8/include -Igen/v8/include -I../../third_party/boringssl/src/include -I../../v8/include -Igen/v8/include -I../../third_party/mesa/src/include -I../../third_party/angle/src/common/third_party/numerics -Igen/angle -I../../third_party/libyuv/include -I../../third_party/re2/src -I../../third_party/zlib -Igen/ui/resources -Igen/ui/resources -I../../third_party/webrtc_overrides -I../../testing/gtest/include -I../../third_party -I../../third_party/webrtc_overrides -I../../third_party -I../../third_party/expat/files/lib -Igen/content/public/android/content_jni_headers -Igen/content/public/android/content_jni_headers/content -Igen/jar_jni/content -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -funwind-tables -fPIC -pipe -fcolor-diagnostics -ffunction-sections -fno-short-enums --target=arm-linux-androideabi -march=armv7-a -mfloat-abi=softfp -mthumb -mtune=generic-armv7-a -mfpu=neon -Wall -Werror -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-deprecated-register -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Os -fno-omit-frame-pointer -gdwarf-3 -g1 --sysroot=../../third_party/android_tools/ndk/platforms/android-16/arch-arm -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Wheader-hygiene -Wstring-conversion -Wexit-time-destructors -Wno-unused-function -fno-threadsafe-statics -fvisibility-inlines-hidden -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++11 -fno-rtti -isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include -isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++abi/libcxxabi/include -isystem../../third_party/android_tools/ndk/sources/android/support/include -fno-exceptions -c ../../content/browser/accessibility/browser_accessibility_manager_android.cc -o obj/content/browser/browser/browser_accessibility_manager_android.o ../../content/browser/accessibility/browser_accessibility_manager_android.cc:702:34: error: no member named 'AccessibilityShowContextMenu' in 'content::BrowserAccessibilityDelegate' node->manager()->delegate()->AccessibilityShowContextMenu(node->GetId()); ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ Original issue's description: > Create AXAction and AXActionData as a way to simplify accessibility actions > > Previously we had a collection of 10 accessibility actions that were > implemented on every accessibility object - all of them implemented for > the web, and about half implemented for aura views and for the automation > extension API. > > This resulted in a lot of boilerplate code because of the many layers of > indirection between the various parts of the codebase. Adding a new > accessibility action meant adding a new IPC and adding a new method to around > a dozen files just to plumb it through. > > In comparison, we have dozens of accessibility events but we handle them all > with a single flexible data structure, so adding a new event doesn't require > so much plumbing. > > This change streamlines accessibility actions too. There's > an enum AXAction with all of the possible accessibility actions, and a > serializable data structure AXActionData that encapsulates the parameters > needed by an accessibility action. > > This cuts down on some duplicate code and reduces the amount of new code > that needs to be written to add support for a new accessibility action in > the future. For example, macOS has "increment" and "decrement" actions for > range controls that we ought to support, and there are some additional > arguments to setAccessibilityFocus that we're contemplating. > > BUG=655272, 655273 > TESTED=Manually triggered each of the supported accessibility actions from > ChromeVox, and from at least one native screen reader. > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation > > Committed: https://crrev.com/daa9dd2ccf9e285d7dd9b389f663f5122506f1f3 > Cr-Commit-Position: refs/heads/master@{#425980} TBR=dtseng@chromium.org,jam@chromium.org,tsepez@chromium.org,dmazzoni@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=655272, 655273 Committed: https://crrev.com/868327e8ad9461c9a07507045db64eabce193fc8 Cr-Commit-Position: refs/heads/master@{#425984}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+506 lines, -472 lines) Patch
M chrome/browser/extensions/api/automation_internal/automation_action_adapter.h View 1 chunk +19 lines, -7 lines 0 comments Download
M chrome/browser/extensions/api/automation_internal/automation_internal_api.cc View 3 chunks +28 lines, -24 lines 0 comments Download
M chrome/browser/ui/aura/accessibility/automation_manager_aura.h View 1 chunk +8 lines, -1 line 0 comments Download
M chrome/browser/ui/aura/accessibility/automation_manager_aura.cc View 2 chunks +27 lines, -38 lines 0 comments Download
M chrome/common/extensions/api/automation.idl View 1 chunk +0 lines, -14 lines 0 comments Download
M chrome/common/extensions/api/automation_internal.idl View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/accessibility/android_granularity_movement_browsertest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/accessibility/browser_accessibility_cocoa.mm View 2 chunks +8 lines, -7 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_manager.h View 4 chunks +18 lines, -13 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_manager.cc View 4 chunks +22 lines, -93 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_manager_android.cc View 2 chunks +6 lines, -2 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_manager_unittest.cc View 1 chunk +17 lines, -1 line 0 comments Download
M content/browser/accessibility/dump_accessibility_events_browsertest.cc View 2 chunks +2 lines, -1 line 0 comments Download
M content/browser/accessibility/hit_testing_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/frame_host/render_frame_host_impl.h View 1 chunk +17 lines, -1 line 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 2 chunks +52 lines, -3 lines 0 comments Download
M content/common/accessibility_messages.h View 3 chunks +49 lines, -19 lines 0 comments Download
M content/public/browser/render_frame_host.h View 2 chunks +11 lines, -7 lines 0 comments Download
M content/renderer/accessibility/render_accessibility_impl.h View 2 chunks +13 lines, -7 lines 0 comments Download
M content/renderer/accessibility/render_accessibility_impl.cc View 5 chunks +205 lines, -72 lines 0 comments Download
M ui/accessibility/BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
M ui/accessibility/PRESUBMIT.py View 1 chunk +0 lines, -2 lines 0 comments Download
D ui/accessibility/ax_action_data.h View 1 chunk +0 lines, -60 lines 0 comments Download
D ui/accessibility/ax_action_data.cc View 1 chunk +0 lines, -62 lines 0 comments Download
M ui/accessibility/ax_enums.idl View 2 chunks +1 line, -33 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
Guido Urdaneta
Created Revert of Create AXAction and AXActionData as a way to simplify accessibility actions
4 years, 2 months ago (2016-10-18 16:20:15 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2430473003/1
4 years, 2 months ago (2016-10-18 16:20:31 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 2 months ago (2016-10-18 16:21:57 UTC) #5
commit-bot: I haz the power
4 years, 2 months ago (2016-10-18 16:24:25 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/868327e8ad9461c9a07507045db64eabce193fc8
Cr-Commit-Position: refs/heads/master@{#425984}

Powered by Google App Engine
This is Rietveld 408576698