Bug 1607276 Comment 12 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Jens Stutte [:jstutte] from comment #11)
> 
> So I take away from this that it is a bad idea to push events during the construction phase of a CCP. And that if we do not do so, everything should be fine, as then the CC can't chip in. It is something to keep in mind and easily violated, though.
> 
Well, it is not about construction, and nothing to do CC as such. It is about refcounting. One needs to keep refcounted objects alive
as long as they need to stay alive. Triggering some JS to run may often end up closing down browser windows or workers or what not, leading to
start shutting down and thus releasing various member variables (even if they are RefPtr. One may just set them to null explicitly).


> 
> I assume, this does not change the behavior but enforces us to keep references in the callee (and helps us to keep in mind the above rule). And that it is not meant to be applied to a constructor.
> 
I don't quite understand this constructor part here. This is about refcounting. When constructor runs, refcnt is 0 initially, but if one wants, explicitly increasing it is of course possible. The caller just needs to know about it.
The normal pattern however is to do something like
RefPtr<MyClass> myObject = new MyClass();
myObject->Init(); // Init may do complicated stuff, and the RefPtr on the stack keeps 'myObject' alive.

One needs to still remember that RefPtr as member variables aren't something which in general keep callee alive.
That is why 
https://searchfox.org/mozilla-central/rev/f98dad153b59a985efd4505912588d4651033395/layout/base/nsDocumentViewer.cpp#1002-1004
is rather common pattern.
(In reply to Jens Stutte [:jstutte] from comment #11)
> 
> So I take away from this that it is a bad idea to push events during the construction phase of a CCP. And that if we do not do so, everything should be fine, as then the CC can't chip in. It is something to keep in mind and easily violated, though.
> 
Well, it is not about construction, and nothing to do CC as such. It is about refcounting. One needs to keep refcounted objects alive
as long as they need to stay alive. Triggering some JS to run may often end up closing down browser windows or workers or what not, leading to
start shutting down and thus releasing various member variables (even if they are RefPtr. One may just set them to null explicitly).


> 
> I assume, this does not change the behavior but enforces us to keep references in the callee (and helps us to keep in mind the above rule). And that it is not meant to be applied to a constructor.
> 
I don't quite understand this constructor part here. This is about refcounting. When constructor runs, refcnt is 0 initially, but if one wants, explicitly increasing it is of course possible. The caller just needs to know about it, so it can be a tad confusing.
The normal pattern however is to do something like
RefPtr<MyClass> myObject = new MyClass();
myObject->Init(); // Init may do complicated stuff, and the RefPtr on the stack keeps 'myObject' alive.

One needs to still remember that RefPtr as member variables aren't something which in general keep callee alive.
That is why 
https://searchfox.org/mozilla-central/rev/f98dad153b59a985efd4505912588d4651033395/layout/base/nsDocumentViewer.cpp#1002-1004
is rather common pattern.
(In reply to Jens Stutte [:jstutte] from comment #11)
> 
> So I take away from this that it is a bad idea to push events during the construction phase of a CCP. And that if we do not do so, everything should be fine, as then the CC can't chip in. It is something to keep in mind and easily violated, though.
> 
Well, it is not about construction, and nothing to do with CC as such. It is about refcounting. One needs to keep refcounted objects alive
as long as they need to stay alive. Triggering some JS to run may often end up closing down browser windows or workers or what not, leading to
start shutting down and thus releasing various member variables (even if they are RefPtr. One may just set them to null explicitly).


> 
> I assume, this does not change the behavior but enforces us to keep references in the callee (and helps us to keep in mind the above rule). And that it is not meant to be applied to a constructor.
> 
I don't quite understand this constructor part here. This is about refcounting. When constructor runs, refcnt is 0 initially, but if one wants, explicitly increasing it is of course possible. The caller just needs to know about it, so it can be a tad confusing.
The normal pattern however is to do something like
RefPtr<MyClass> myObject = new MyClass();
myObject->Init(); // Init may do complicated stuff, and the RefPtr on the stack keeps 'myObject' alive.

One needs to still remember that RefPtr as member variables aren't something which in general keep callee alive.
That is why 
https://searchfox.org/mozilla-central/rev/f98dad153b59a985efd4505912588d4651033395/layout/base/nsDocumentViewer.cpp#1002-1004
is rather common pattern.

Back to Bug 1607276 Comment 12