Last Comment Bug 720647 - Add message managers to black-bit-propagation
: Add message managers to black-bit-propagation
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: 12 Branch
: All All
: -- normal (vote)
: mozilla12
Assigned To: Olli Pettay [:smaug]
:
:
Mentors:
Depends on:
Blocks: 705582
  Show dependency treegraph
 
Reported: 2012-01-24 02:25 PST by Olli Pettay [:smaug]
Modified: 2012-01-26 13:18 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (5.39 KB, patch)
2012-01-24 02:25 PST, Olli Pettay [:smaug]
continuation: review+
Details | Diff | Splinter Review
patch (5.22 KB, patch)
2012-01-26 07:29 PST, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
patch (4.82 KB, patch)
2012-01-26 07:39 PST, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review

Description Olli Pettay [:smaug] 2012-01-24 02:25:38 PST
Created attachment 591047 [details] [diff] [review]
patch

This code will be used by nsCCUncollectableMarker::Observe.
Comment 1 Olli Pettay [:smaug] 2012-01-24 02:27:45 PST
Comment on attachment 591047 [details] [diff] [review]
patch

Er, this is only partial patch. Uploading a new one in a minute.
Comment 2 Olli Pettay [:smaug] 2012-01-24 02:35:14 PST
Comment on attachment 591047 [details] [diff] [review]
patch

Actually this is ok. I'll move some other stuff to nsCCUncollectableMarker
Comment 3 Andrew McCreight [:mccr8] 2012-01-24 04:40:18 PST
Did you mean to make nsIInProcessContentFrameMessageManager builtinclass? You changed the UUID but didn't do anything else to that class.
Comment 4 Andrew McCreight [:mccr8] 2012-01-24 04:42:38 PST
Looks reasonable to me, though I still don't like how you are doing the unmark gray on the XPCWrappedJS here. ;)  But let's hash that out in bug 720536.
Comment 5 Olli Pettay [:smaug] 2012-01-24 05:33:55 PST
(In reply to Andrew McCreight [:mccr8] from comment #3)
> Did you mean to make nsIInProcessContentFrameMessageManager builtinclass?
> You changed the UUID but didn't do anything else to that class.
It isn't a scriptable interface, so builtinclass doesn't really matter, and uuid is changed
by the script which changes uuids in all the interfaces which inherit an interface which got
changed.
Comment 6 Andrew McCreight [:mccr8] 2012-01-24 06:31:10 PST
Ah, okay, that makes sense.  I figured there was a reason, I just wasn't sure what it was.
Comment 7 Andrew McCreight [:mccr8] 2012-01-25 14:57:35 PST
Comment on attachment 591047 [details] [diff] [review]
patch

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

r=me if you revert the xpcprivate.h include and replace the if block as described below.

::: content/base/src/nsFrameMessageManager.cpp
@@ +1117,5 @@
> +  PRUint32 len = mListeners.Length();
> +  for (PRUint32 i = 0; i < len; ++i) {
> +    nsCOMPtr<nsIXPConnectWrappedJS> wjs =
> +      do_QueryInterface(mListeners[i].mListener);
> +    if (wjs) {

I assume you will replace this whole "if (wjs) { ... }" block with just "xpc_UnmarkGrayObject(wjs);" as per the other bugs.
Comment 8 Olli Pettay [:smaug] 2012-01-26 01:58:50 PST
Yup. I have that already fixed locally.
Comment 9 Olli Pettay [:smaug] 2012-01-26 07:29:57 PST
Created attachment 591791 [details] [diff] [review]
patch
Comment 10 Olli Pettay [:smaug] 2012-01-26 07:39:04 PST
Created attachment 591796 [details] [diff] [review]
patch
Comment 11 Olli Pettay [:smaug] 2012-01-26 12:30:13 PST
https://hg.mozilla.org/mozilla-central/rev/85b19db37fd0

Note You need to log in before you can comment on or make changes to this bug.