nsAccUtils::IsTextInterfaceSupportCorrect() is too expensive and called too often in DEBUG

RESOLVED FIXED in mozilla16

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: hub, Assigned: hub)

Tracking

(Blocks: 1 bug)

Trunk
mozilla16
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
nsAccUtils::IsTextInterfaceSupportCorrect() is too expensive and called too often in DEBUG

Bug 750191 made it build unconditionally in DEBUG which is why we get it all the time.

It is called when querying the role, on all platform but I noticed it on Mac.

Updated

5 years ago
Blocks: 389800
(Assignee)

Updated

5 years ago
Blocks: 772060
No longer blocks: 772060
(Assignee)

Comment 1

5 years ago
it makes DEBUG unusable on Mac.
(Assignee)

Comment 2

5 years ago
Created attachment 641278 [details] [diff] [review]
nsAccUtils::IsTextInterfaceSupportCorrect() is no longer called in DEBUG on Mac. r=
(Assignee)

Updated

5 years ago
Assignee: nobody → hub
(Assignee)

Updated

5 years ago
Attachment #641278 - Flags: review?(dbolter)
Comment on attachment 641278 [details] [diff] [review]
nsAccUtils::IsTextInterfaceSupportCorrect() is no longer called in DEBUG on Mac. r=

>-#ifdef DEBUG
>+#ifdef DEBUG_a11y

unless someone wants to own getting rid of all the one off DEBUG_FOO around, and make sure DEBUG_A11Y keeps building I'd rather not add uses of DEBUG_A11Y.

what's wrong with making IsTextInterfaceSupportCorrect() faster? it doesn't look terribly hard to make atleast a bit faster, but I'm not clear on how often its called on mac.

Also do you actually ever build with DEBUG_A11Y? or would it make sense to just remove this all together?
(Assignee)

Comment 4

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #3)
> Comment on attachment 641278 [details] [diff] [review]
> nsAccUtils::IsTextInterfaceSupportCorrect() is no longer called in DEBUG on
> Mac. r=
> 
> >-#ifdef DEBUG
> >+#ifdef DEBUG_a11y
> 
> unless someone wants to own getting rid of all the one off DEBUG_FOO around,
> and make sure DEBUG_A11Y keeps building I'd rather not add uses of
> DEBUG_A11Y.
> 
> what's wrong with making IsTextInterfaceSupportCorrect() faster? it doesn't
> look terribly hard to make atleast a bit faster,

I tend to just think that debug only code is debug only code, then I shouldn't bother.

> but I'm not clear on how
> often its called on mac.

It is called each time we call -[mozAccessible role], which is more often than not. Now maybe I should just call it somewhere else.

> Also do you actually ever build with DEBUG_A11Y? or would it make sense to
> just remove this all together?

I had never, but bug 750191 did cause this to come into the picture.
(In reply to Hub Figuiere [:hub] from comment #4)
> (In reply to Trevor Saunders (:tbsaunde) from comment #3)
> > Comment on attachment 641278 [details] [diff] [review]
> > nsAccUtils::IsTextInterfaceSupportCorrect() is no longer called in DEBUG on
> > Mac. r=
> > 
> > >-#ifdef DEBUG
> > >+#ifdef DEBUG_a11y
> > 
> > unless someone wants to own getting rid of all the one off DEBUG_FOO around,
> > and make sure DEBUG_A11Y keeps building I'd rather not add uses of
> > DEBUG_A11Y.
> > 
> > what's wrong with making IsTextInterfaceSupportCorrect() faster? it doesn't
> > look terribly hard to make atleast a bit faster,
> 
> I tend to just think that debug only code is debug only code, then I
> shouldn't bother.

well, its mostly deCOMtamination stuff which needs to happen eventually anyway...

> > Also do you actually ever build with DEBUG_A11Y? or would it make sense to
> > just remove this all together?
> 
> I had never, but bug 750191 did cause this to come into the picture.

ok, it wouldn't suprise me if it doesn't build then.
(Assignee)

Comment 6

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #5)

> > > what's wrong with making IsTextInterfaceSupportCorrect() faster? it doesn't
> > > look terribly hard to make atleast a bit faster,
> > 
> > I tend to just think that debug only code is debug only code, then I
> > shouldn't bother.
> 
> well, its mostly deCOMtamination stuff which needs to happen eventually
> anyway...

So maybe I can just get away with removing it? I'm fine with that.
Comment on attachment 641278 [details] [diff] [review]
nsAccUtils::IsTextInterfaceSupportCorrect() is no longer called in DEBUG on Mac. r=

Review of attachment 641278 [details] [diff] [review]:
-----------------------------------------------------------------

If this helps your mac debugging, fine with me.

::: accessible/src/mac/mozAccessible.mm
@@ +411,5 @@
>  {
>    if (!mGeckoAccessible)
>      return nil;
>  
> +#ifdef DEBUG_a11y

It should be DEBUG_A11Y, all caps.
Attachment #641278 - Flags: review?(dbolter) → review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/aec3f38712a3
https://hg.mozilla.org/mozilla-central/rev/aec3f38712a3
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16

Comment 10

5 years ago
I prefer something like DEBUG_DISABLED to stress out what you do and file a bug to sort out IsTextInterfaceSupportCorrect things.
You need to log in before you can comment on or make changes to this bug.