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

Issue 2981083002: Migrate BrowserAccessibility windows unique id handling to AXPlatformNodeWin. (Closed)

Created:
3 years, 5 months ago by dougt
Modified:
3 years, 4 months ago
Reviewers:
dmazzoni
CC:
chromium-reviews, aboxhall+watch_chromium.org, nektar+watch_chromium.org, jam, yuzo+watch_chromium.org, dougt+watch_chromium.org, darin-cc_chromium.org, aleventhal+watch_chromium.org, dtseng+watch_chromium.org, je_julie, dmazzoni+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Migrate BrowserAccessibility windows unique id handling to AXPlatformNodeWin. Global identifiers are allocated out of ui::GetNextAXPlatformNodeUniqueId currently. Both BrowserAccessibility and the AXPlatform code both independently Assign a unique id during object creation. What we want to have happen is for the BrowserAccessibilityComWin class, and it�s typical owner, the BrowserAccessibilityWin, to share the same unique id. This makes it easier to reason about as well as necessary to be able to Implement get_uniqueID(). This CL makes private the |unique_id_| used in both above base classes private and forces all access through virtual class methods. The BrowserAccessibilityWin overrides and implements in terms of AXPlatformNodeWin. BUG=703369 Review-Url: https://codereview.chromium.org/2981083002 Cr-Commit-Position: refs/heads/master@{#489177} Committed: https://chromium.googlesource.com/chromium/src/+/8b5355784ac527145920d375a2065537c76dea01

Patch Set 1 #

Patch Set 2 : Android fix. I was hoping I could get rid of this. Someday though! #

Total comments: 1

Patch Set 3 : Different approach #

Patch Set 4 : Build Android #

Patch Set 5 : Use after free no more #

Unified diffs Side-by-side diffs Delta from patch set Stats (+137 lines, -116 lines) Patch
M content/browser/accessibility/browser_accessibility.h View 1 2 2 chunks +0 lines, -3 lines 0 comments Download
M content/browser/accessibility/browser_accessibility.cc View 1 2 3 chunks +1 line, -29 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_android.h View 1 2 3 3 chunks +6 lines, -0 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_android.cc View 1 2 3 3 chunks +27 lines, -1 line 0 comments Download
M content/browser/accessibility/browser_accessibility_com_win.h View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/accessibility/browser_accessibility_com_win.cc View 1 2 3 4 8 chunks +11 lines, -17 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_event.cc View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/accessibility/browser_accessibility_manager_android.cc View 1 2 3 3 chunks +14 lines, -12 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_manager_win.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M content/browser/accessibility/browser_accessibility_win_unittest.cc View 1 2 9 chunks +24 lines, -12 lines 0 comments Download
M content/browser/accessibility/web_contents_accessibility_android.cc View 1 2 3 7 chunks +22 lines, -9 lines 0 comments Download
M ui/accessibility/platform/ax_platform_node.h View 1 2 1 chunk +0 lines, -4 lines 0 comments Download
M ui/accessibility/platform/ax_platform_node.cc View 1 2 1 chunk +1 line, -25 lines 0 comments Download
M ui/accessibility/platform/ax_platform_node_win.h View 1 2 2 chunks +4 lines, -0 lines 0 comments Download
M ui/accessibility/platform/ax_platform_node_win.cc View 1 2 3 4 3 chunks +26 lines, -1 line 0 comments Download

Messages

Total messages: 32 (25 generated)
dougt
Hey Dominic, ptal.
3 years, 5 months ago (2017-07-16 21:45:52 UTC) #10
dmazzoni
One idea is below. Another idea is to make GetUniqueId() part of the AXPlatformNodeDelegate interface. ...
3 years, 5 months ago (2017-07-17 08:38:20 UTC) #11
dougt
dmazzoni, i just pushed the unique id handling into AXPlatformNodeWin and BrowserAccessibilityAndroid and stopped believing ...
3 years, 5 months ago (2017-07-20 21:03:31 UTC) #25
dmazzoni
lgtm That works! Thanks.
3 years, 5 months ago (2017-07-24 23:05:15 UTC) #26
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/2981083002/80001
3 years, 5 months ago (2017-07-24 23:20:37 UTC) #28
commit-bot: I haz the power
Committed patchset #4 (id:80001) as https://chromium.googlesource.com/chromium/src/+/8b5355784ac527145920d375a2065537c76dea01
3 years, 5 months ago (2017-07-25 01:30:06 UTC) #31
dougt
3 years, 5 months ago (2017-07-25 16:29:35 UTC) #32
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:80001) has been created in
https://codereview.chromium.org/2988753002/ by dougt@chromium.org.

The reason for reverting is: Revert due to crbug 748472. Will reland w/ fix
after merge to M61 branch..

Powered by Google App Engine
This is Rietveld 408576698