The default bug view has changed. See this FAQ.

make FocusManager::FocusedDOMNode faster

RESOLVED FIXED in mozilla15

Status

()

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

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla15
access
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

2.75 KB, patch
tbsaunde
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Created attachment 625022 [details] [diff] [review]
patch

It takes up to 3% in example of bug 732872 because these nsCOMPtr stuffs.
Attachment #625022 - Flags: review?(trev.saunders)
Attachment #625022 - Flags: review?(bugs)

Comment 1

5 years ago
Comment on attachment 625022 [details] [diff] [review]
patch

>diff --git a/accessible/src/base/FocusManager.cpp b/accessible/src/base/FocusManager.cpp
>--- a/accessible/src/base/FocusManager.cpp
>+++ b/accessible/src/base/FocusManager.cpp
>@@ -373,25 +373,18 @@ FocusManager::FocusedDOMNode() const
>   // keeps the focus.
>   if (focusedElm) {
>     if (nsEventStateManager::IsRemoteTarget(focusedElm))
>       return nsnull;
>     return focusedElm;
>   }
> 
>   // Otherwise the focus can be on DOM document.
>-  nsCOMPtr<nsIDOMWindow> focusedWnd;
>-  DOMFocusManager->GetFocusedWindow(getter_AddRefs(focusedWnd));
>-  if (focusedWnd) {
>-    nsCOMPtr<nsIDOMDocument> DOMDoc;
>-    focusedWnd->GetDocument(getter_AddRefs(DOMDoc));
>-    nsCOMPtr<nsIDocument> DOMDocNode(do_QueryInterface(DOMDoc));
>-    return DOMDocNode;
>-  }
>-  return nsnull;
>+  nsPIDOMWindow* focusedWnd = DOMFocusManager->GetFocusedWindow();
>+  return focusedWnd ? focusedWnd->GetExtantDocumentNode() : nsnull;
> }
Hmm, you're changing behavior here. GetDocument() isn't the same 
as GetExtant*. GetDocument() may create the document.
Is that ok for a11y?



>+  nsIDocument* GetExtantDocumentNode() const
Maybe GetExtantDoc()
Thank you for adding this. I've been thinking this for a long time.

r+ for the dom/ parts
Attachment #625022 - Flags: review?(bugs) → review+

Comment 2

5 years ago
Update the iid of nsPIDOMWindow
(Assignee)

Comment 3

5 years ago
(In reply to Olli Pettay [:smaug] from comment #1)

> Hmm, you're changing behavior here. GetDocument() isn't the same 
> as GetExtant*. GetDocument() may create the document.
> Is that ok for a11y?

yes, no DOM document means no accessible what means this method shouldn't called.
(Assignee)

Comment 4

5 years ago
Created attachment 625644 [details] [diff] [review]
patch2
Attachment #625022 - Attachment is obsolete: true
Attachment #625022 - Flags: review?(trev.saunders)
Attachment #625644 - Flags: review?(dbolter)
Comment on attachment 625644 [details] [diff] [review]
patch2

sorry I was away so much... but I'm freer now, so figured I'd steel this back :)
Attachment #625644 - Flags: review?(dbolter) → review+
(Assignee)

Comment 6

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #5)
> Comment on attachment 625644 [details] [diff] [review]
> patch2
> 
> sorry I was away so much... but I'm freer now, so figured I'd steel this
> back :)

cool, welcome back :)
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/219dc3f47568
Target Milestone: --- → mozilla15
Comment on attachment 625644 [details] [diff] [review]
patch2

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

cool
https://hg.mozilla.org/mozilla-central/rev/219dc3f47568
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.