841608 - Assertion: Uh, inner window set as event target! with FireMozTimeChangeEvent

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Gregor Wagner [:gwagner]]

Breakpoint 1, NS_DebugBreak_P (aSeverity=1, aStr=0x436b9afc "Uh, inner window set as event target!", aExpr=0x436b9b24 "!win || !win->IsInnerWindow()", 
    aFile=0x436b95b8 "/Volumes/2mac/gaia/3src/content/events/src/nsDOMEvent.cpp", aLine=801) at /Volumes/2mac/gaia/3src/xpcom/base/nsDebugImpl.cpp:293
293	     sevString = "###!!! ASSERTION";
(gdb) bt
#0  NS_DebugBreak_P (aSeverity=1, aStr=0x436b9afc "Uh, inner window set as event target!", aExpr=0x436b9b24 "!win || !win->IsInnerWindow()", 
    aFile=0x436b95b8 "/Volumes/2mac/gaia/3src/content/events/src/nsDOMEvent.cpp", aLine=801) at /Volumes/2mac/gaia/3src/xpcom/base/nsDebugImpl.cpp:293
#1  0x41306974 in nsDOMEvent::SetTarget (this=0x4a9fed60, aTarget=0x404ee200) at /Volumes/2mac/gaia/3src/content/events/src/nsDOMEvent.cpp:800
#2  0x41146a5e in GetEventAndTarget (aDoc=0x46d60c00, aTarget=0x404ee210, aEventName=..., aCanBubble=true, aCancelable=false, aTrusted=true, aEvent=0xbea40ab0, aTargetOut=0xbea40aac)
    at /Volumes/2mac/gaia/3src/content/base/src/nsContentUtils.cpp:3506
#3  0x41146c26 in nsContentUtils::DispatchEvent (aDoc=0x46d60c00, aTarget=0x404ee210, aEventName=..., aCanBubble=true, aCancelable=false, aTrusted=true, aDefaultAction=0x0)
    at /Volumes/2mac/gaia/3src/content/base/src/nsContentUtils.cpp:3547
#4  0x41146b70 in nsContentUtils::DispatchTrustedEvent (aDoc=0x46d60c00, aTarget=0x404ee210, aEventName=..., aCanBubble=true, aCancelable=false, aDefaultAction=0x0)
    at /Volumes/2mac/gaia/3src/content/base/src/nsContentUtils.cpp:3522
#5  0x417220cc in nsSystemTimeChangeObserver::FireMozTimeChangeEvent (this=0x4a83b4d0) at /Volumes/2mac/gaia/3src/dom/time/TimeChangeObserver.cpp:61
#6  0x417221e8 in nsSystemTimeChangeObserver::Notify (this=0x4a83b4d0, aClockDeltaMS=@0xbea40bc0) at /Volumes/2mac/gaia/3src/dom/time/TimeChangeObserver.cpp:77
#7  0x421b3872 in mozilla::ObserverList<long long>::Broadcast (this=0x4a55037c, aParam=@0xbea40bc0) at ../dist/include/mozilla/Observer.h:67
#8  0x421b29da in mozilla::hal::ObserversManager<long long>::BroadcastInformation (this=0x44663660, aInfo=@0xbea40bc0) at /Volumes/2mac/gaia/3src/hal/Hal.cpp:226
#9  0x421b0aec in mozilla::hal::NotifySystemClockChange (aClockDeltaMS=@0xbea40bc0) at /Volumes/2mac/gaia/3src/hal/Hal.cpp:453
#10 0x421bb93e in mozilla::hal_impl::AdjustSystemClock (aDeltaMilliseconds=334) at /Volumes/2mac/gaia/3src/hal/gonk/GonkHal.cpp:748
#11 0x421b0c0a in mozilla::hal::AdjustSystemClock (aDeltaMilliseconds=334) at /Volumes/2mac/gaia/3src/hal/Hal.cpp:494
#12 0x41721ab6 in mozilla::dom::time::TimeService::Set (this=0x4a0b3670, aTimeInMS=1360888353947) at /Volumes/2mac/gaia/3src/dom/time/TimeService.cpp:32
#13 0x4267688a in NS_InvokeByIndex_P (that=0x4a0b3670, methodIndex=3, paramCount=<value optimized out>, params=<value optimized out>)
    at /Volumes/2mac/gaia/3src/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp:160
#14 0x41b4d586 in CallMethodHelper::Invoke (ccx=..., mode=XPCWrappedNative::CALL_METHOD) at /Volumes/2mac/gaia/3src/js/xpconnect/src/XPCWrappedNative.cpp:3085
#15 CallMethodHelper::Call (ccx=..., mode=XPCWrappedNative::CALL_METHOD) at /Volumes/2mac/gaia/3src/js/xpconnect/src/XPCWrappedNative.cpp:2419
#16 XPCWrappedNative::CallMethod (ccx=..., mode=XPCWrappedNative::CALL_METHOD) at /Volumes/2mac/gaia/3src/js/xpconnect/src/XPCWrappedNative.cpp:2385
#17 0x41b5873e in XPC_WN_CallMethod (cx=0x40440700, argc=1, vp=0x4596a150) at /Volumes/2mac/gaia/3src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1417
#18 0x42ddafde in CallJSNative (cx=0x40440700, args=..., construct=js::NO_CONSTRUCT) at /Volumes/2mac/gaia/3src/js/src/jscntxtinlines.h:327
#19 js::InvokeKernel (cx=0x40440700, args=..., construct=js::NO_CONSTRUCT) at /Volumes/2mac/gaia/3src/js/src/jsinterp.cpp:367
#20 0x42e036f4 in js::Interpret (cx=0x40440700, entryFrame=0x4596a060, interpMode=js::JSINTERP_NORMAL) at /Volumes/2mac/gaia/3src/js/src/jsinterp.cpp:2344
#21 0x42dda9a2 in js::RunScript (cx=0x40440700, fp=0x4596a060) at /Volumes/2mac/gaia/3src/js/src/jsinterp.cpp:324
#22 0x42ddb142 in js::InvokeKernel (cx=0x40440700, args=..., construct=js::NO_CONSTRUCT) at /Volumes/2mac/gaia/3src/js/src/jsinterp.cpp:381
#23 0x42cd2cb8 in js::Invoke (cx=0x40440700, args=..., construct=js::NO_CONSTRUCT) at /Volumes/2mac/gaia/3src/js/src/jsinterp.h:135
#24 0x42d7712e in js::CallOrConstructBoundFunction (cx=0x40440700, argc=1, vp=0x4596a030) at /Volumes/2mac/gaia/3src/js/src/jsfun.cpp:1099
#25 0x42ddafde in CallJSNative (cx=0x40440700, args=..., construct=js::NO_CONSTRUCT) at /Volumes/2mac/gaia/3src/js/src/jscntxtinlines.h:327
#26 js::InvokeKernel (cx=0x40440700, args=..., construct=js::NO_CONSTRUCT) at /Volumes/2mac/gaia/3src/js/src/jsinterp.cpp:367
#27 0x42cd2cb8 in js::Invoke (cx=0x40440700, args=..., construct=js::NO_CONSTRUCT) at /Volumes/2mac/gaia/3src/js/src/jsinterp.h:135
#28 0x42ddb60c in js::Invoke (cx=0x40440700, thisv=..., fval=..., argc=1, argv=0xbea45e58, rval=0xbea45e50) at /Volumes/2mac/gaia/3src/js/src/jsinterp.cpp:414
#29 0x42cc5072 in JS_CallFunctionValue (cx=0x40440700, objArg=0x46228100, fval=..., argc=1, argv=0xbea45e58, rval=0xbea45e50) at /Volumes/2mac/gaia/3src/js/src/jsapi.cpp:5737

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 714421 [details] [diff] [review]
Patch, v1

(Builds on top of the patch in bug 841612)

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 714421 [details] [diff] [review]
Patch, v1

GetDoc() should probably be GetExtantDoc().

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

This should probably block since we're not sure exactly what the bad consequences are and it's a simple patch. At best we have |event.target !== window| in the page receiving the event, at worst we expose an inner window to JS (super bad).

Comment 5

[Alex Keybl [:akeybl]]

We'll approve a low risk nomination, but this isn't a blocker without known user impact.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

I was looking at the code some more, and nsEventDispatcher ensures that right kind
of target is used during dispatch. So, in this case setting .target to wrong window shouldn't
be bad.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Oops, I cleared some flags.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 714421 [details] [diff] [review]
Patch, v1

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

::: dom/time/TimeChangeObserver.cpp
@@ +44,4 @@
>      nsCOMPtr<nsIDocument> document;
> +    if (!innerWindow ||
> +        !(document = innerWindow->GetDoc()) ||
> +        !(outerWindow = innerWindow->GetOuterWindow())) {

Does this need to also check that inner window is the current inner?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

It doesn't. Event isn't handled on non-current-inner-window anyway.

Comment 10

[Ben Turner (not reading bugmail, use the needinfo flag!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f125f034e222

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f125f034e222

Comment 12

[Gregor Wagner [:gwagner]]

can we push this to b2g18? it's tracking.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/b5af46889a81