Last Comment Bug 768997 - nsAccUtils::IsTextInterfaceSupportCorrect() is too expensive and called too often in DEBUG
: nsAccUtils::IsTextInterfaceSupportCorrect() is too expensive and called too o...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: mozilla16
Assigned To: Hubert Figuiere [:hub]
:
Mentors:
Depends on:
Blocks: cleana11y
  Show dependency treegraph
 
Reported: 2012-06-27 11:29 PDT by Hubert Figuiere [:hub]
Modified: 2012-07-16 01:18 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
nsAccUtils::IsTextInterfaceSupportCorrect() is no longer called in DEBUG on Mac. r= (952 bytes, patch)
2012-07-11 17:23 PDT, Hubert Figuiere [:hub]
dbolter: review+
Details | Diff | Review

Description Hubert Figuiere [:hub] 2012-06-27 11:29:18 PDT
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.
Comment 1 Hubert Figuiere [:hub] 2012-07-11 17:21:20 PDT
it makes DEBUG unusable on Mac.
Comment 2 Hubert Figuiere [:hub] 2012-07-11 17:23:50 PDT
Created attachment 641278 [details] [diff] [review]
nsAccUtils::IsTextInterfaceSupportCorrect() is no longer called in DEBUG on Mac. r=
Comment 3 Trevor Saunders (:tbsaunde) 2012-07-11 22:17:28 PDT
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?
Comment 4 Hubert Figuiere [:hub] 2012-07-11 22:28:33 PDT
(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.
Comment 5 Trevor Saunders (:tbsaunde) 2012-07-12 00:01:11 PDT
(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.
Comment 6 Hubert Figuiere [:hub] 2012-07-12 12:17:15 PDT
(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 7 David Bolter [:davidb] 2012-07-12 12:32:46 PDT
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.
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-07-13 20:21:52 PDT
https://hg.mozilla.org/mozilla-central/rev/aec3f38712a3
Comment 10 alexander :surkov 2012-07-16 01:18:51 PDT
I prefer something like DEBUG_DISABLED to stress out what you do and file a bug to sort out IsTextInterfaceSupportCorrect things.

Note You need to log in before you can comment on or make changes to this bug.