Closed
Bug 1413072
Opened 8 years ago
Closed 8 years ago
[e10s a11y] Eliminate pointless cross-process QueryInterface for IAccessibleText
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla58
People
(Reporter: Jamie, Assigned: Jamie)
Details
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
MarcoZ
:
review+
ritu
:
approval-mozilla-beta-
|
Details |
AccessibleHandler's AccessibleTextTearoff currently makes separate queries for the IAccessibleText and IAccessibleHypertext interfaces. However, IAccessibleHypertext inherits from IAccessibleText, and wherever one of these interfaces are present, Gecko always implements both. NVDA (and possibly other clients) always queries for both interfaces when rendering. Therefore, we should just query for IAccessibleHypertext for all of these methods, thus saving a cross-process call should a client want both interfaces. There is no penalty if a client only wants one of them, so we only have something to gain and nothing to lose here.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jteh
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•8 years ago
|
||
Win64 build: https://public-artifacts.taskcluster.net/e1QK3zpOQambcSf4bFgYig/0/public/build/target.zip
Marco, if you have a chance, it'd be good if you could compare this against nightly to see if it yields a noticeable perf improvement. If it does, we should request uplift, as it is a fairly simple patch.
Comment 3•8 years ago
|
||
This try build behaves as if bug 1363595 and friends never landed, e. g. World War I is back up from 8 to 12-13 seconds.
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8924380 [details]
Bug 1413072: Eliminate pointless cross-process QueryInterface for IAccessibleText in AccessibleHandler.
https://reviewboard.mozilla.org/r/195660/#review200858
This looks good to me.
Attachment #8924380 -
Flags: review?(mzehe) → review+
Comment 5•8 years ago
|
||
Note: The reason I was seeing bad perf on the try ZIP was because it wasn't using the handler. I manually registered it and found that it is snappier than current Nightly especially on small pages, not so much a perf difference on big ones. I believe this is a good and safe optimization also for 57.
Pushed by mzehe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1116fde20e3b
Eliminate pointless cross-process QueryInterface for IAccessibleText in AccessibleHandler. r=MarcoZ
Comment 7•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8924380 [details]
Bug 1413072: Eliminate pointless cross-process QueryInterface for IAccessibleText in AccessibleHandler.
Approval Request Comment
[Feature/Bug causing the regression]: E10s Windows accessibility.
[User impact if declined]: Decreased performance.
[Is this code covered by automated tests?]: No, because we don't have a mechanism for platform accessibility testing.
[Has the fix been verified in Nightly?]: I confirmed that pages render as expected with equivalent or better performance.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: Simple patch containing a safe optimisation. Easy to verify that breakage has not occurred.
[String changes made/needed]: None.
Attachment #8924380 -
Flags: approval-mozilla-beta?
status-firefox57:
--- → affected
Comment on attachment 8924380 [details]
Bug 1413072: Eliminate pointless cross-process QueryInterface for IAccessibleText in AccessibleHandler.
Jim weighed in on this uplift and we don't believe this is a must fix for 57. I'd prefer to let this one ride the 58 train.
Attachment #8924380 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in
before you can comment on or make changes to this bug.
Description
•