Comment 124 for bug 239831

Revision history for this message
In , Enn (enndeakin) wrote :

(In reply to comment #100)
> The lines of "NOISE: nsContentUtils::CanCallerAccess Called" are the log of
> nsContentUtils::CanCallerAccess called (I put printf() to the patch).
>
> It was called 32 times per a cycle.

Is that calls to nsContentUtils::CanCallerAccess or calls from the focus manager to it? When I run the normal performance test (not sure about this specific one) I get only one call to ShiftFocusInner.

What happens if you just replace the call to nsContentUtils::CanCallerAccess with code that just always matches/never matches?

I suspect more likely that removing a focus call is causing some other behaviour to occur, or changing the timing of a flush or somesuch. The pageload performance tests don't actually test focus time, they only test up until the page is loaded, and focus can occur after that point. So using the pageload tests for testing focus performance regressions is generally going to be inaccurate anyway.

I don't really like this newer patch as it adds quite a bit of extra complexity and requires callers to think about what to set the new flag to.

That said, I noticed a problem with the earlier patch anyway with the following test:

<textarea id="t">...</textarea><script>document.getElementById('t').focus()</script>

Focus the urlbar, reload the page, lower the window and then raise it again. The urlbar should be focused yet the textarea is now focused, even though the urlbar was focused before the window was lowered. This is caused because the setting of 'canStealFocus' should actually just be clearing 'allowFrameSwitch', so that the later conditions which check this don't match.

Personally I would rather just fix this problem, and check in the earlier patch.