730013 - nsTransactionManager (and undo/redo stacks) are often in CC graph, even if the manager is alive

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

The worst case is probably locationbar, where we may get quite large undo/redo stacks,
and lots of transactions.
It is a bit difficult to add any CanSkip methods to the manager, since the ownership
rules aren't that clear. (the manager is used also elsewhere than just with editor).
So, the patch just makes us addref/release less so that txmanager doesn't end up to
purple buffer in the most common case (this makes the code also a tiny bit faster).
The API of nsIEditor gets also a bit better, since it already has undo(cnt)/redo(cnt), but
no way to tell how many times undo/redo is effective.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

https://tbpl.mozilla.org/?tree=Try&rev=c8359a91ef94

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

This might be good for FF12 too, since the patch should be very safe, and it reduces
CCs times during typing to locationbar.

Comment 3

[(no longer active)]

Comment on attachment 600073 [details] [diff] [review]
patch

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

This is really good, and I'd be fine if you would want to backport it to branches.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/13b571bde26a

Comment 5

[Alex Keybl [:akeybl]]

(In reply to Olli Pettay [:smaug] from comment #2)
> This might be good for FF12 too, since the patch should be very safe, and it
> reduces
> CCs times during typing to locationbar.

We haven't seen reports of pausing in the locationbar - could this be related to other pauses as well?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Yes. The patch should help with CC runs which happen when user is typing to
location bar or to any input/textarea element.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 600073 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): N/A 
User impact if declined: Possibly higher CC times while typing
Testing completed (on m-c, etc.): Landed m-c 2012-02-23
Risk to taking this patch (and alternatives if risky):
  Should be very low risk patch
String changes made by this patch: N/A

Comment 8

[Alex Keybl [:akeybl]]

Comment on attachment 600073 [details] [diff] [review]
patch

[Triage Comment]
Approved for Aurora 12 since this is low risk and we have heard of slow input into text boxes being an intermittent issue for our users.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: aurora

https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=cbfa968b61f5

Comment 10

[u279076]

Is this something QA can verify?