Closed Bug 628922 Opened 9 years ago Closed 9 years ago

layout should use cached nsIAccessibilityService

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b11

People

(Reporter: surkov, Assigned: surkov)

References

Details

(Keywords: access, perf)

Attachments

(2 files, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
That allows us to create accessible tree and make layout notifications to a11y faster.
Attachment #507057 - Flags: superreview?(roc)
Attachment #507057 - Flags: review?(bolterbugz)
Attachment #507057 - Flags: approval2.0?
Comment on attachment 507057 [details] [diff] [review]
patch

r=me, the logic looks right to me. Maybe not a concern, but MOZ_ENABLE_LIBXUL is not true for Thunderbird right?

>+#ifdef MOZ_ENABLE_LIBXUL
>+    return GetAccService();
>+#else
>+    if (gIsAccessibilityActive) {
>+      nsCOMPtr<nsIAccessibilityService> srv =
>+        do_GetService("@mozilla.org/accessibilityService;1");
>+      return srv;
>+    }
>+    return nsnull;
>+#endif
Attachment #507057 - Flags: review?(bolterbugz) → review+
+#ifdef ACCESSIBILITY
+#include "nsServiceManagerUtils.h"
+#include "nsAccessibilityService.h"
+#endif
+

Can we not do this, and just make AccService() non-inline?
Attached patch patch2Splinter Review
fix Robert's comment, use nsAccessibilityService directly
Attachment #507057 - Attachment is obsolete: true
Attachment #507384 - Flags: superreview?(roc)
Attachment #507384 - Flags: approval2.0?
Attachment #507057 - Flags: superreview?(roc)
Attachment #507057 - Flags: approval2.0?
Attachment #507384 - Flags: superreview?(roc)
Attachment #507384 - Flags: superreview+
Attachment #507384 - Flags: approval2.0?
Attachment #507384 - Flags: approval2.0+
Blocks: a11yperf_fx4
Keywords: perf
landed on 2.0 beta11 - http://hg.mozilla.org/mozilla-central/rev/cb3fcbce2ef4
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b11
return static_cast<nsAccessibilityService*>(srv);

This line failed with Solaris Studio.

return static_cast<nsAccessibilityService*>(srv.get());

will work.
Attached patch solaris fixSplinter Review
Attachment #507783 - Flags: review?(roc)
Attachment #507783 - Flags: approval2.0?
(In reply to comment #6)
> return static_cast<nsAccessibilityService*>(srv);
> 
> This line failed with Solaris Studio.
> 
> return static_cast<nsAccessibilityService*>(srv.get());
> 
> will work.

Thanks, Ginn! Does non libxul build is used on Solaris?
Not for release build. Sometimes I use non libxul debug build to save recompiling time.
Depends on: 629612
Attachment #507783 - Flags: review?(roc)
Attachment #507783 - Flags: review+
Attachment #507783 - Flags: approval2.0?
Attachment #507783 - Flags: approval2.0+
Comment on attachment 507783 [details] [diff] [review]
solaris fix

FWIW, this is not a Solaris specific fix, this is a C++ language fix!  :-)
(In reply to comment #11)
> Comment on attachment 507783 [details] [diff] [review]
> solaris fix
> 
> FWIW, this is not a Solaris specific fix, this is a C++ language fix!  :-)

I got it but after some time :) Just Ginn reports Solaris specific bugs from time to time and I thought nsCOMPtr should have overridden casting (perhaps it has but it's used for implicit casting only).
You need to log in before you can comment on or make changes to this bug.