Persona is no longer an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 744034 - TI disabled in compartments created for iframes
: TI disabled in compartments created for iframes
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Bobby Holley (:bholley) (busy with Stylo)
: Andrew Overholt [:overholt]
Depends on: 744153 745483 745661
Blocks: cpg
  Show dependency treegraph
Reported: 2012-04-10 09:15 PDT by Bobby Holley (:bholley) (busy with Stylo)
Modified: 2012-04-15 19:10 PDT (History)
15 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Link the script context to the outer window earlier to ensure that we always have TI for content. v3 (5.86 KB, patch)
2012-04-11 12:02 PDT, Bobby Holley (:bholley) (busy with Stylo)
mrbkap: review+
Details | Diff | Splinter Review

Description Bobby Holley (:bholley) (busy with Stylo) 2012-04-10 09:15:23 PDT
See bug 650353 comment 62.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2012-04-10 09:17:29 PDT
Is this specific to dromaeo, or does it happen with any same-origin iframe?
Comment 2 Bobby Holley (:bholley) (busy with Stylo) 2012-04-10 09:19:39 PDT
(In reply to Boris Zbarsky (:bz) from comment #1)
> Is this specific to dromaeo, or does it happen with any same-origin iframe?

I'm guessing the latter, but I wanted to make the bug title honest to the information at hand. Once luke confirms, we can change it.
Comment 3 Luke Wagner [:luke] 2012-04-10 10:43:35 PDT
(In reply to Boris Zbarsky (:bz) from comment #1)
Note specific to Dromaeo at all.  If I browse around, I see TI-disabled compartments created all the time.  I also put a printf in a trunk browser and it seems to happen when a new compartment is created for an iframe (so, for cross-origin iframes).

In reply to bug 650353 comment 64:

#0  xpc_CreateGlobalObject (cx=0x7fffcd26b000, clasp=0x7ffff6d5abc8, 
    principal=0x7fffcc8f52c0, ptr=0x0, wantXrays=false, global=0x7fffffff52d8, 
    compartment=0x7fffffff52e0) at /moz/mc/js/xpconnect/src/nsXPConnect.cpp:1202
#1  0x00007ffff3026047 in XPCWrappedNative::WrapNewGlobal (ccx=..., nativeHelper=..., 
    principal=0x7fffcc8f52c0, initStandardClasses=false, wrappedGlobal=0x7fffffff5550)
    at /moz/mc/js/xpconnect/src/XPCWrappedNative.cpp:362
#2  0x00007ffff2fcd744 in nsXPConnect::InitClassesWithNewWrappedGlobal (
    this=0x7fffe3ae1bb0, aJSContext=0x7fffcd26b000, aCOMObj=0x7fffccdc5078, 
    aPrincipal=0x7fffcc8f52c0, aFlags=0, _retval=0x7fffffff5600)
    at /moz/mc/js/xpconnect/src/nsXPConnect.cpp:1259
#3  0x00007ffff2a79df3 in nsJSContext::CreateNativeGlobalForInner (this=0x7fffcb57bc10, 
    aNewInner=0x7fffccdc5078, aIsChrome=false, aPrincipal=0x7fffcc8f52c0, 
    aNativeGlobal=0x7fffccdc5230, aHolder=0x7fffccadcdd0)
    at /moz/mc/dom/base/nsJSEnvironment.cpp:2070
#4  0x00007ffff2a9aa59 in nsGlobalWindow::SetNewDocument (this=0x7fffccadcc00, aDocument=
    0x7fffcd7e1800, aState=0x0, aForceReuseInnerWindow=false)
    at /moz/mc/dom/base/nsGlobalWindow.cpp:2096
#5  0x00007ffff2328962 in DocumentViewerImpl::InitInternal (this=0x7fffcc6c6680, 
    aParentWidget=0x0, aState=0x0, aBounds=..., aDoCreation=true, aNeedMakeCX=true, 
    aForceSetNewDocument=true) at /moz/mc/layout/base/nsDocumentViewer.cpp:980
#6  0x00007ffff23274db in DocumentViewerImpl::Init (this=0x7fffcc6c6680, 
    aParentWidget=0x0, aBounds=...) at /moz/mc/layout/base/nsDocumentViewer.cpp:723
#7  0x00007ffff3149c05 in nsDocShell::SetupNewViewer (this=0x7fffccadb400, 
    aNewViewer=0x7fffcc6c6680) at /moz/mc/docshell/base/nsDocShell.cpp:7749
#8  0x00007ffff3141e9b in nsDocShell::Embed (this=0x7fffccadb400, 
    aContentViewer=0x7fffcc6c6680, aCommand=0x7ffff44644f2 "", aExtraInfo=0x0)
    at /moz/mc/docshell/base/nsDocShell.cpp:5847
#9  0x00007ffff31448ea in nsDocShell::CreateAboutBlankContentViewer (this=0x7fffccadb400, 
    aPrincipal=0x7fffcc8f52c0, aBaseURI=0x7fffcb72e900, aTryToSaveOldPresentation=true)
    at /moz/mc/docshell/base/nsDocShell.cpp:6576
#10 0x00007ffff314433a in nsDocShell::EnsureContentViewer (this=0x7fffccadb400)
    at /moz/mc/docshell/base/nsDocShell.cpp:6469
#11 0x00007ffff312f421 in nsDocShell::GetInterface (this=0x7fffccadb400, aIID=..., 
    aSink=0x7fffffff62e8) at /moz/mc/docshell/base/nsDocShell.cpp:956
#12 0x00007ffff37b9a3c in nsGetInterface::operator() (this=0x7fffffff6330, aIID=..., 
    at /moz/mc/objdbg/xpcom/build/nsIInterfaceRequestorUtils.cpp:52
#13 0x00007ffff26e1f4d in nsCOMPtr<nsIDOMDocument>::assign_from_helper (
    this=0x7fffffff6350, helper=..., aIID=...) at ../../../dist/include/nsCOMPtr.h:1232
#14 0x00007ffff26e0fd7 in nsCOMPtr<nsIDOMDocument>::nsCOMPtr (this=0x7fffffff6350, 
    helper=...) at ../../../dist/include/nsCOMPtr.h:632
#15 0x00007ffff2a9dd5d in nsGlobalWindow::GetDocument (this=0x7fffccadcc00, 
    aDocument=0x7fffffff6380) at /moz/mc/dom/base/nsGlobalWindow.cpp:2957
#16 0x00007ffff2a91fc5 in nsPIDOMWindow::EnsureInnerWindow (this=0x7fffccadcc00)
    at /moz/mc/dom/base/nsPIDOMWindow.h:353
#17 0x00007ffff2a92221 in nsGlobalWindow::WrapObject (this=0x7fffccadcc00, 
    cx=0x7fffd56e8400, scope=0x7fffd79f2060, triedToWrap=0x7fffffff653e)
    at /moz/mc/dom/base/nsGlobalWindow.h:288
#18 0x00007ffff2ff21cc in XPCConvert::NativeInterface2JSObject (lccx=..., 
    d=0x7fffffff68c0, dest=0x0, aHelper=..., iid=0x7fffffff6a20, Interface=0x0, 
    allowNativeWrapper=true, pErr=0x7fffffff68e4)
    at /moz/mc/js/xpconnect/src/XPCConvert.cpp:931
#19 0x00007ffff2ff075e in XPCConvert::NativeData2JS (lccx=..., d=0x7fffffff68c0, 
    s=0x7fffffff6b40, type=..., iid=0x7fffffff6a20, pErr=0x7fffffff68e4)
    at /moz/mc/js/xpconnect/src/XPCConvert.cpp:360
#20 0x00007ffff2fefacc in XPCConvert::NativeData2JS (ccx=..., d=0x7fffffff68c0, 
    s=0x7fffffff6b40, type=..., iid=0x7fffffff6a20, pErr=0x7fffffff68e4)
    at /moz/mc/js/xpconnect/src/xpcprivate.h:3228
#21 0x00007ffff302cd19 in CallMethodHelper::GatherAndConvertResults (this=0x7fffffff6b00)
    at /moz/mc/js/xpconnect/src/XPCWrappedNative.cpp:2630
#22 0x00007ffff302c192 in CallMethodHelper::Call (this=0x7fffffff6b00)
    at /moz/mc/js/xpconnect/src/XPCWrappedNative.cpp:2440
#23 0x00007ffff302bfae in XPCWrappedNative::CallMethod (ccx=..., 
    at /moz/mc/js/xpconnect/src/XPCWrappedNative.cpp:2393
#24 0x00007ffff3033f60 in XPCWrappedNative::GetAttribute (ccx=...)
    at /moz/mc/js/xpconnect/src/xpcprivate.h:2618
#25 0x00007ffff303938a in XPC_WN_GetterSetter (cx=0x7fffd56e8400, argc=0, 
---Type <return> to continue, or q <return> to quit---
    vp=0x7fffdeb00448) at /moz/mc/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1597
#26 0x00007ffff3e22a2b in js::CallJSNative (cx=0x7fffd56e8400, 
    native=0x7ffff303908d <XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*)>, 
    args=...) at /moz/mc/js/src/jscntxtinlines.h:314
#27 0x00007ffff3e28bff in js::InvokeKernel (cx=0x7fffd56e8400, args=..., 
    construct=js::NO_CONSTRUCT) at /moz/mc/js/src/jsinterp.cpp:516
#28 0x00007ffff3d840cf in js::Invoke (cx=0x7fffd56e8400, args=..., 
    construct=js::NO_CONSTRUCT) at /moz/mc/js/src/jsinterp.h:172
#29 0x00007ffff3e28e77 in js::Invoke (cx=0x7fffd56e8400, thisv=..., fval=..., argc=0, 
    argv=0x0, rval=0x7fffffff7530) at /moz/mc/js/src/jsinterp.cpp:563
#30 0x00007ffff3e29274 in js::InvokeGetterOrSetter (cx=0x7fffd56e8400, 
    obj=0x7fffccccb580, fval=..., argc=0, argv=0x0, rval=0x7fffffff7530)
    at /moz/mc/js/src/jsinterp.cpp:637
#31 0x00007ffff3e4ffa1 in js::Shape::get (this=0x7fffcccb6240, cx=0x7fffd56e8400, 
    receiver=0x7fffccccb580, obj=0x7fffccccb580, pobj=0x7fffccccb580, vp=0x7fffffff7530)
    at /moz/mc/js/src/jsscopeinlines.h:286
#32 0x00007ffff3e5f98b in js_NativeGetInline (cx=0x7fffd56e8400, receiver=0x7fffccccb580, 
    obj=0x7fffccccb580, pobj=0x7fffccccb580, shape=0x7fffcccb6240, getHow=1, 
    vp=0x7fffffff7530) at /moz/mc/js/src/jsobj.cpp:4943
#33 0x00007ffff3e600f0 in js_GetPropertyHelperInline (cx=0x7fffd56e8400, 
    obj=0x7fffccccb580, receiver=0x7fffccccb580, id=..., getHow=1, vp=0x7fffffff7530)
    at /moz/mc/js/src/jsobj.cpp:5093
#34 0x00007ffff3e6016b in js::GetPropertyHelper (cx=0x7fffd56e8400, obj=0x7fffccccb580, 
    id=..., getHow=1, vp=0x7fffffff7530) at /moz/mc/js/src/jsobj.cpp:5102
#35 0x00007ffff3e25627 in js::GetPropertyOperation (cx=0x7fffd56e8400, 
    pc=0x7fffd54332b7 "5", lval=..., vp=0x7fffffff7530)
    at /moz/mc/js/src/jsinterpinlines.h:264
#36 0x00007ffff3e34430 in js::Interpret (cx=0x7fffd56e8400, entryFrame=0x7fffdeb003a8, 
    interpMode=js::JSINTERP_NORMAL) at /moz/mc/js/src/jsinterp.cpp:2575
#37 0x00007ffff3e28892 in js::RunScript (cx=0x7fffd56e8400, script=0x7fffd58398d0, 
    fp=0x7fffdeb003a8) at /moz/mc/js/src/jsinterp.cpp:472
#38 0x00007ffff3e28cbf in js::InvokeKernel (cx=0x7fffd56e8400, args=..., 
    construct=js::NO_CONSTRUCT) at /moz/mc/js/src/jsinterp.cpp:531
#39 0x00007ffff3d840cf in js::Invoke (cx=0x7fffd56e8400, args=..., 
    construct=js::NO_CONSTRUCT) at /moz/mc/js/src/jsinterp.h:172
#40 0x00007ffff3e28e77 in js::Invoke (cx=0x7fffd56e8400, thisv=..., fval=..., argc=4, 
    argv=0x7fffffff8700, rval=0x7fffffff83e0) at /moz/mc/js/src/jsinterp.cpp:563
#41 0x00007ffff3d774d2 in JS_CallFunctionValue (cx=0x7fffd56e8400, obj=0x7fffd58a48c0, 
    fval=..., argc=4, argv=0x7fffffff8700, rval=0x7fffffff83e0)
    at /moz/mc/js/src/jsapi.cpp:5404
#42 0x00007ffff3021445 in nsXPCWrappedJSClass::CallMethod (this=0x7fffd5268d80, wrapper=
    0x7fffd5291900, methodIndex=3, info=0x7fffe3c3d918, nativeParams=0x7fffffff8830)
    at /moz/mc/js/xpconnect/src/XPCWrappedJSClass.cpp:1517
#43 0x00007ffff3017c93 in nsXPCWrappedJS::CallMethod (this=0x7fffd5291900, methodIndex=3, 
    info=0x7fffe3c3d918, params=0x7fffffff8830)
    at /moz/mc/js/xpconnect/src/XPCWrappedJS.cpp:617
#44 0x00007ffff3851105 in PrepareAndDispatch (self=0x7fffd56505c0, methodIndex=3, 
    args=0x7fffffff89b0, gpregs=0x7fffffff8930, fpregs=0x7fffffff8960)
    at /moz/mc/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:153
#45 0x00007ffff38502c7 in SharedStub () from /moz/mc/objdbg/dist/bin/
#46 0x00007ffff322a827 in nsBrowserStatusFilter::OnStateChange (this=0x7fffd5235980, 
    aWebProgress=0x7fffccadb428, aRequest=0x7fffccaeb3c0, aStateFlags=983041, aStatus=0)
    at /moz/mc/toolkit/components/statusfilter/nsBrowserStatusFilter.cpp:183
#47 0x00007ffff3171e4d in nsDocLoader::DoFireOnStateChange (this=0x7fffd5436c00, 
    aProgress=0x7fffccadb428, aRequest=0x7fffccaeb3c0, aStateFlags=@0x7fffffff8b04, 
    aStatus=0) at /moz/mc/uriloader/base/nsDocLoader.cpp:1383
#48 0x00007ffff3171b4c in nsDocLoader::FireOnStateChange (this=0x7fffccadb400, 
    aProgress=0x7fffccadb428, aRequest=0x7fffccaeb3c0, aStateFlags=983041, aStatus=0)
    at /moz/mc/uriloader/base/nsDocLoader.cpp:1320
#49 0x00007ffff3170806 in nsDocLoader::doStartDocumentLoad (this=0x7fffccadb400)
    at /moz/mc/uriloader/base/nsDocLoader.cpp:885
#50 0x00007ffff316fcd2 in nsDocLoader::OnStartRequest (this=0x7fffccadb400, 
    request=0x7fffccaeb3c0, aCtxt=0x0) at /moz/mc/uriloader/base/nsDocLoader.cpp:582
#51 0x00007ffff202e36e in nsLoadGroup::AddRequest (this=0x7fffcd180bb0, 
    request=0x7fffccaeb3c0, ctxt=0x0) at /moz/mc/netwerk/base/src/nsLoadGroup.cpp:612
#52 0x00007ffff200b729 in nsBaseChannel::AsyncOpen (this=0x7fffccaeb370, 
    listener=0x7fffcd844c90, ctxt=0x0) at /moz/mc/netwerk/base/src/nsBaseChannel.cpp:625
#53 0x00007ffff316cc79 in nsURILoader::OpenURI (this=0x7fffdcda18e0, 
---Type <return> to continue, or q <return> to quit---
    channel=0x7fffccaeb3c0, aIsContentPreferred=false, aWindowContext=0x7fffccadb430)
    at /moz/mc/uriloader/base/nsURILoader.cpp:815
#54 0x00007ffff314f961 in nsDocShell::DoChannelLoad (this=0x7fffccadb400, 
    aChannel=0x7fffccaeb3c0, aURILoader=0x7fffdcda18e0, aBypassClassifier=false)
    at /moz/mc/docshell/base/nsDocShell.cpp:9151
#55 0x00007ffff314f1b5 in nsDocShell::DoURILoad (this=0x7fffccadb400, 
    aURI=0x7fffcc5b9900, aReferrerURI=0x7fffcb72e800, aSendReferrer=true, 
    aOwner=0x7fffcc8f52c0, aTypeHint=0x0, aPostData=0x0, aHeadersData=0x0, 
    aFirstParty=false, aDocShell=0x0, aRequest=0x7fffffff9530, aIsNewWindowTarget=false, 
    aBypassClassifier=false, aForceAllowCookies=false)
    at /moz/mc/docshell/base/nsDocShell.cpp:8988
#56 0x00007ffff314dc54 in nsDocShell::InternalLoad (this=0x7fffccadb400, 
    aURI=0x7fffcc5b9900, aReferrer=0x7fffcb72e800, aOwner=0x7fffcc8f52c0, aFlags=0, 
    aWindowTarget=0x7fffcb52d8d8, aTypeHint=0x0, aPostData=0x0, aHeadersData=0x0, 
    aLoadType=1, aSHEntry=0x0, aFirstParty=false, aDocShell=0x0, aRequest=0x0)
    at /moz/mc/docshell/base/nsDocShell.cpp:8679
#57 0x00007ffff31315c4 in nsDocShell::LoadURI (this=0x7fffccadb400, aURI=0x7fffcc5b9900, 
    aLoadInfo=0x7fffcc8f67c0, aLoadFlags=0, aFirstParty=false)
    at /moz/mc/docshell/base/nsDocShell.cpp:1496
#58 0x00007ffff26d934b in nsFrameLoader::ReallyStartLoadingInternal (this=0x7fffcb57bac0)
    at /moz/mc/content/base/src/nsFrameLoader.cpp:491
#59 0x00007ffff26d8e60 in nsFrameLoader::ReallyStartLoading (this=0x7fffcb57bac0)
    at /moz/mc/content/base/src/nsFrameLoader.cpp:432
#60 0x00007ffff26b12b1 in nsDocument::MaybeInitializeFinalizeFrameLoaders (
    this=0x7fffcd262000) at /moz/mc/content/base/src/nsDocument.cpp:5492
#61 0x00007ffff26ab6c3 in nsDocument::EndUpdate (this=0x7fffcd262000, aUpdateType=1)
    at /moz/mc/content/base/src/nsDocument.cpp:4035
#62 0x00007ffff29661b8 in nsHTMLDocument::EndUpdate (this=0x7fffcd262000, aUpdateType=1)
    at /moz/mc/content/html/document/src/nsHTMLDocument.cpp:2273
#63 0x00007ffff240f3f3 in mozAutoDocUpdate::~mozAutoDocUpdate (this=0x7fffffff9c10, 
    __in_chrg=<optimized out>)
    at /moz/mc/layout/generic/../../content/base/src/mozAutoDocUpdate.h:67
#64 0x00007ffff26fcc4a in nsINode::ReplaceOrInsertBefore (this=0x7fffcb7c4670, 
    aReplace=false, aNewChild=0x7fffccdf9110, aRefChild=0x0)
    at /moz/mc/content/base/src/nsGenericElement.cpp:4237
#65 0x00007ffff274b11f in nsINode::ReplaceOrInsertBefore (this=0x7fffcb7c4670, 
    aReplace=false, aNewChild=0x7fffccdf9110, aRefChild=0x0, aReturn=0x7fffffffa10c)
    at ../../../dist/include/nsINode.h:1463
#66 0x00007ffff274b0da in nsINode::InsertBefore (this=0x7fffcb7c4670, 
    aNewChild=0x7fffccdf9110, aRefChild=0x0, aReturn=0x7fffffffa10c)
    at ../../../dist/include/nsINode.h:506
#67 0x00007ffff2871b60 in nsINode::AppendChild (this=0x7fffcb7c4670, 
    aNewChild=0x7fffccdf9110, aReturn=0x7fffffffa10c)
    at ../../../../dist/include/nsINode.h:516
#68 0x00007ffff305fcd4 in nsIDOMNode_AppendChild (cx=0x7fffd56e8400, argc=1, 
    vp=0x7fffdeb002f0) at /moz/mc/objdbg/js/xpconnect/src/dom_quickstubs.cpp:5498
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2012-04-10 11:15:20 PDT
So yeah, this is broken.

What happens is that for the subframe we create a docshell.  This calls nsDocShell::EnsureScriptEnvironment which ends up calling nsJSContext::nsJSContext.  This creates a new JSContext, grabs its current options, adds the PRIVATE_IS_NSISUPPORTS flag registers a callback for pref changes to "js_options_dot_str", and moves on.

The next thing that happens is that nsJSContext::InitContext is called.  This calls JSOptionChangedCallback manually.  The code in there calls GetGlobalObject() on the nsJSContext, but this is null because we haven't gotten that far yet.  And then it does:

  bool useTypeInference = !chromeWindow && contentWindow &&

but since we have no global contentWindow is null, and we end up with false.  We set the JSContext options accordingly.

The next thing that happens is that in nsGlobalWindow::SetNewDocument we do this:

2087          rv = mContext->CreateNativeGlobalForInner(sgo, isChrome,
2088                                                    aDocument->NodePrincipal(),
2089                                                    &newInnerWindow->mJSObject,
2090                                                    getter_AddRefs(holder));

where mContext is the nsJSContext.  This lands us in xpc_CreateGlobalObject with the JSContext we set up earlier, still with the TI-less options.  So in the new world we create the new compartment, and don't allow TI in there.

Then we call nsJSContext::SetGlobalObject via the call to CreateOuterObject() in SetNewDocument.  This just sets the member and does nothing else.

Then we call nsJSContext::InitClasses from SetNewDocument.  This also calls JSOptionChangedCallback, which now sets up our context options correctly.  But of course this is not propagated to the compartment...  Future loads in this subframe will end up working, of course.

The "right" fix is probably to SetGlobalObject either before InitContext() if we can or at least before CreateNativeGlobalForInner (and in the latter case, call JSOptionChangedCallback from inside SetGlobalObject).

Another option (possibly less risky) would be to somehow update the relevant compartment flags inside that InitClasses call...
Comment 5 Bobby Holley (:bholley) (busy with Stylo) 2012-04-11 12:02:18 PDT
Created attachment 614093 [details] [diff] [review]
Link the script context to the outer window earlier to ensure that we always have TI for content. v3

Attaching a patch, flagging blake for review.

This had a green try run:
Comment 6 Johnny Stenback (:jst, 2012-04-11 12:13:00 PDT
*Nice* find guys!
Comment 7 Blake Kaplan (:mrbkap) 2012-04-11 19:44:12 PDT
Comment on attachment 614093 [details] [diff] [review]
Link the script context to the outer window earlier to ensure that we always have TI for content. v3

Review of attachment 614093 [details] [diff] [review]:

::: js/xpconnect/src/nsXPConnect.cpp
@@ +1202,5 @@
> +    // Make sure that Type Inference is enabled for everything non-chrome.
> +    // Sandboxes and compilation scopes are exceptions. See bug 744034.
> +    mozilla::DebugOnly<bool> isSystem;
> +    mozilla::DebugOnly<nsIScriptSecurityManager*> ssm;
> +    MOZ_ASSERT_IF(strcmp(clasp->name, "Sandbox") &&

Does this need to be a fatal assert if it trips?
Comment 8 :Ms2ger (⌚ UTC+1/+2) 2012-04-12 03:01:27 PDT
MOZ_ASSERT is fatal.
Comment 9 Bobby Holley (:bholley) (busy with Stylo) 2012-04-12 11:17:29 PDT
(In reply to Ms2ger from comment #8)
> MOZ_ASSERT is fatal.

I think blake was asking whether we should make it nonfatal.

The answer is that it doesn't really need to be fatal, but it might as well be. I had to make it fatal in order to make sure that we got all the cases in the mochitest suite (since mochitests don't go orange for NS_ASSERTIONS). At this point, we might as well just keep it. Also, from an aesthetic standpoint, there's no NS_ASSERT_IF. ;-)
Comment 10 Bobby Holley (:bholley) (busy with Stylo) 2012-04-12 11:22:43 PDT
Pushed to m-i:
Comment 11 Luke Wagner [:luke] 2012-04-12 15:15:12 PDT
/me thinks that all new assertions should be fatal.  How else do you find out when your assumptions are wrong?
Comment 12 Blake Kaplan (:mrbkap) 2012-04-13 02:31:06 PDT
Luke, this question comes up from time to time... Is it worth turning the tree orange for missed optimization opportunities? I'm not entirely convinced that it is, but I'd rather spot problems and fix them than let them go unfound (therefore the r+). I was just asking, was all.
Comment 13 Marco Bonardo [::mak] 2012-04-13 04:27:17 PDT
Comment 14 Luke Wagner [:luke] 2012-04-13 10:43:50 PDT
Good question.  In general, I am all for asserting things that seem like they should be true as long as we can be reasonably sure they won't be a cause of random orange.  For example, TI can become disabled (even when the pref is enabled) if there is an OOM, so you'd have to assert pretty "early" before the first possible OOM.  But perhaps instead we could change how TI/jits get enabled/disabled so that it isn't so convoluted ;)

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