FormFillController holds XPCOM objects too long

RESOLVED FIXED in Camino2.0

Status

Camino Graveyard
General
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Stuart Morgan, Assigned: Stuart Morgan)

Tracking

({fixed1.8.1.22})

Trunk
Camino2.0
All
Mac OS X
fixed1.8.1.22

Details

Attachments

(1 attachment)

fix
1.64 KB, patch
Mike Pinkerton (not reading bugmail)
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
Like 440002, we need to be more careful about releasing core objects before XPCOM shuts down. We have a bunch of warnings on quit in debug builds, and several of them trace back to FormFillController and KeychainAutoCompleteSession session having core objects as members that stay until dealloc.

I'm not sure yet whether I'll put a listener in KACS as well, or just have each FFC release its KACS early, since it's the only owners of them.
Flags: camino1.6.7?
Would this (help) fix bug 480330, or is that more complicated?
(Assignee)

Comment 2

9 years ago
I don't think it would, since that crash is from trying to talk to another one of our own objects based on a core trigger; this is kind of the reverse.
(Assignee)

Comment 3

9 years ago
Created attachment 366069 [details] [diff] [review]
fix

Rather than sprinkle shutdown listeners everywhere, I just had BrowserWrapper take care of tearing down the whole formfill stack, since all FormFillControllers are BrowserWrapper-owned.
Attachment #366069 - Flags: superreview?(mikepinkerton)
Comment on attachment 366069 [details] [diff] [review]
fix

sr=pink
Attachment #366069 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Comment 5

9 years ago
Landed on CVS trunk.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Stuart, this patch appears to cause us to leak more Gecko stuff when quitting with a username child window visible.

STR:
1) Launch Camino to about:blank in a fresh profile
2) http://mail.google.com (which redirects to the Google account login)
3) Select the Keychain-filled username and delete it, then down-arrow to bring up the child window
4) Cmd-Q while the child window is still visible

Pre-patch:

 nsStringStats
  => mAllocCount:           9265
  => mReallocCount:          952
  => mFreeCount:            8605  --  LEAKED 660 !!!
  => mShareCount:           4288
  => mAdoptCount:            711
  => mAdoptFreeCount:        709  --  LEAKED 2 !!!

Post-patch:

 nsStringStats
  => mAllocCount:          10511
  => mReallocCount:          992
  => mFreeCount:            9755  --  LEAKED 756 !!!
  => mShareCount:           5524
  => mAdoptCount:            836
  => mAdoptFreeCount:        834  --  LEAKED 2 !!!

For poor comparison purposes, a launch-about:blank-shutdown usually shows this:

 nsStringStats
  => mAllocCount:           6039
  => mReallocCount:          270
  => mFreeCount:            6031  --  LEAKED 8 !!!
  => mShareCount:           1686
  => mAdoptCount:            325
  => mAdoptFreeCount:        325

I have similar numbers on branch (we're much more leaky there on my about:blank baseline to begin with).
Per the meeting, landed on the MOZILLA_1_8_BRANCH as-is; I'll spin the leak stuff in comment 6 off into a new bug.
Flags: camino1.6.7? → camino1.6.7+
Keywords: fixed1.8.1.21
Keywords: fixed1.8.1.21 → fixed1.8.1.22
You need to log in before you can comment on or make changes to this bug.