633672 - quora.com bloats out of control (part 2)

Status: RESOLVED DUPLICATE of bug 633382

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

Description

[Andreas Gal :gal]

+++ This bug was initially created as a clone of Bug #630072 +++

This bug will investigate the remaining quora.com leak.

Comment 1

[Andreas Gal :gal]

Andreas, I'm still seeing the InstallTrigger getter hold an old window alive
with a slightly different path:

        --[InstallTrigger getter]-> 0x7fffcd03bcc0 [JS Object (Proxy)
(global=7fffcd066a20)]
        --[wrappedObject]-> 0x7fffd21ea4d0 [JS Object (Function)
(global=7fffdeb35f30)]
        --[upvars[0]]-> 0x7fffdebba0d0 [JS Object (Proxy)
(global=7fffdaa6c0d8)]
        --[proto]-> 0x7fffdaa93340 [JS Object (Proxy) (global=7fffdaa6c0d8)]
        --[wrappedObject]-> 0x7fffd475d108 [JS Object
(XPC_WN_ModsAllowed_NoCall_Proto_JSClass - Window) (global=7fffd475b120)]
        --[parent]-> 0x7fffd475b120 [JS Object (Window) (global=7fffd475b120)]
        --[xpc_GetJSPrivate(obj)]-> 0x7fffe1cb3ac0 [XPCWrappedNative (Window)]
        --[mIdentity]-> 0x7fffd46a5078 [nsGlobalWindow]

Comment 2

[Andreas Gal :gal]

This is still InstallTrigger, so its still a regression and a hardblocker. There is a 3rd part to this that is an editor leak which is probably not a regression, so that would be a softblocker. We have to fix this first to tell whats up.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Is there a bug on the editor thing?  Peter and I were talking about it today and have a good idea for what will fix it...

Comment 4

[Peter Van der Beken [:peterv]]

Again:

0x7fffcfc0e9d8 [JS Object (Window) (global=7fffcfc0e9d8)]
--[InstallTrigger getter]-> 0x7fffcfc2b440 [JS Object (Proxy) (global=7fffcfc0e9d8)]
--[wrappedObject]-> 0x7fffe2af1d10 [JS Object (Function) (global=7fffdeb35f30)]
--[upvars[0]]-> 0x7fffdebba0d0 [JS Object (Proxy) (global=7fffe0734510)]
--[proto]-> 0x7fffdebf5d00 [JS Object (Proxy) (global=7fffe0734510)]
--[proto]-> 0x7fffdebf5c98 [JS Object (Proxy) (global=7fffe0734510)]
--[wrappedObject]-> 0x7fffe2a061b0 [JS Object (Object) (global=7fffe2a06120)]
--[parent]-> 0x7fffe2a06120 [JS Object (Window) (global=7fffe2a06120)]
--[xpc_GetJSPrivate(obj)]-> 0x7fffe4034040 [XPCWrappedNative (Window)]
--[mIdentity]-> 0x7fffd4ccd478 [nsGlobalWindow]

Starting at the object stored in upvars[0]:

(gdb) p (JSObject*)0x7fffdebba0d0
$10 = (JSObject *) 0x7fffdebba0d0
(gdb) call js_DumpObject($)
object 0x7fffdebba0d0
class 0x7ffff7b6e8c0 Proxy
flags: delegate
not native
proto <Proxy object at 0x7fffdebf5d00>
parent <ChromeWindow object at 0x7fffe0734510>
slots:

(gdb) x/a $->getProxyHandler()
0x7ffff7b945e0 <_ZN3xpc11XrayWrapperI25JSCrossCompartmentWrapperE9singletonE>:	0x7ffff7a47290 <_ZTVN3xpc11XrayWrapperI25JSCrossCompartmentWrapperEE+16>
(gdb) p $->getProxyPrivate().toObjectOrNull()
$11 = (JSObject *) 0x7fffdebb5068
(gdb) call js_DumpObject($)
object 0x7fffdebb5068
class 0x7ffff7b6ea00 Proxy
flags: delegate
not native
proto <XPC_WN_ModsAllowed_NoCall_Proto_JSClass object at 0x7fffcfc2c370>
parent <Window object at 0x7fffcfc0e9d8>
slots:

(gdb) x/a $->getProxyHandler()
0x7ffff7b8ed30 <_ZN18nsOuterWindowProxy9singletonE>:	0x7ffff79c2f90 <_ZTV18nsOuterWindowProxy+16>

So looks like 0x7fffdebba0d0 is a wrapper around an outer window. The proto of the outer window is 0x7fffcfc2c370

If I go up the proto chain for the proxy:

(gdb) p $10->proto
$12 = (JSObject *) 0x7fffdebf5d00
(gdb) x/a $->getProxyHandler()
0x7ffff7b94640 <_ZN3xpc15NoWaiverWrapper9singletonE>:	0x7ffff7a46a30 <_ZTVN3xpc15NoWaiverWrapperE+16>
(gdb) p $->getProxyPrivate().toObjectOrNull()
$13 = (JSObject *) 0x7fffdebb3108

The proto of the wrapper is a wrapper around a different object than the proto of the outer window.

(gdb) call js_DumpObject($)
object 0x7fffdebb3108
class 0x7ffff7b52f60 XPC_WN_ModsAllowed_NoCall_Proto_JSClass
flags: delegate own_shape inDictionaryMode hasPropertyTable
properties:
    "__defineSetter__": slot 11
    "__defineGetter__": slot 10
    "__lookupSetter__": slot 9
    "__lookupGetter__": slot 8
    enumerate "dump": slot 7
    enumerate "scrollByLines": slot 6
    enumerate "getSelection": slot 5
    enumerate shared "top": slot -1
    enumerate shared "parent": slot -1
    enumerate shared "name": slot -1
    enumerate "addEventListener": slot 4
    enumerate "removeEventListener": slot 3
    enumerate "dispatchEvent": slot 2
    enumerate "getComputedStyle": slot 1
    enumerate shared "localStorage": slot -1
    enumerate shared "globalStorage": slot -1
    enumerate shared "sessionStorage": slot -1
proto <Object at 0x7fffe2a061b0>
parent <Window object at 0x7fffe2a06120>
private 0x7fffe1c99b40
slots:
   0 (reserved) = undefined
   1 = <function getComputedStyle at 0x7fffdebb2400 (JSFunction at 0x7fffdebb2400)>
   2 = <function dispatchEvent at 0x7fffdebb2480 (JSFunction at 0x7fffdebb2480)>
   3 = <function removeEventListener at 0x7fffdebb2500 (JSFunction at 0x7fffdebb2500)>
   4 = <function addEventListener at 0x7fffdebb2580 (JSFunction at 0x7fffdebb2580)>
   5 = <function getSelection at 0x7fffdebb2780 (JSFunction at 0x7fffdebb2780)>
   6 = <function scrollByLines at 0x7fffdebb2800 (JSFunction at 0x7fffdebb2800)>
   7 = <function dump at 0x7fffdebb2700 (JSFunction at 0x7fffdebb2700)>
   8 = <function __lookupGetter__ at 0x7fffdebb2880 (JSFunction at 0x7fffdebb2880)>
   9 = <function __lookupSetter__ at 0x7fffdebb2900 (JSFunction at 0x7fffdebb2900)>
  10 = <function __defineGetter__ at 0x7fffdebb2980 (JSFunction at 0x7fffdebb2980)>
  11 = <function __defineSetter__ at 0x7fffdebb2a00 (JSFunction at 0x7fffdebb2a00)>

Comment 5

[Peter Van der Beken [:peterv]]

I found the issue. I've been hunting for the place where we changed the proto of the window objects, but after a lot of printf debugging and coming up with nothing it hit me that we're actually not doing that, we're transplanting. Turns out that when we transplant same-compartment we don't do anything about cross-compartment wrappers in other compartments. It makes sense, since we really replace the guts of the existing object with a new one and cross-compartment wrappers can keep pointing to the existing object. However, transplanting means that the proto of the object changes (it's part of the guts) so we change the proto to one that's parented to a different object, but the proto of the cross-compartment wrappers aren't updated. Once I added code to go through all cross-comparment wrappers and update their proto to a cross-compartment wrapper of the new proto the leak went away. I'll attach my patch.
Gal, does this make sense?

Comment 6

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 7

[Johnny Stenback (:jst)]

Excellent find Peter! I think we probably need the same treatment in js_TransplantObjectWithWrapper() (landed just a few days back), no?

Comment 8

[Andreas Gal :gal]

peterv, I think the proper fix for this is bug 633382. If you still see this edge, please re-open this bug.

Comment 9

[Peter Van der Beken [:peterv]]

Yeah, seems fixed now.