Open Bug 1297714 Opened 8 years ago Updated 2 years ago

nsDocumentViewer::SetFullZoom can be called twice causing assertion error in debug

Categories

(Core :: Layout, defect)

defect

Tracking

()

People

(Reporter: zer0, Unassigned, NeedInfo)

Details

Working on bug 1241867 I got an exception about `SetFullZoom` called twice.

That is happening because something as the following code in the unit test:

    mql.addListener(function listener() {
      ok("MediaQueryList's listener for zoom invoked.");
      mql2.removeListener(listener);

      setFullZoom(originalZoom);

      done();
    });

    setFullZoom(zoom);

It seems that the media query listeners are called synchronously, so the `SetFullZoom` happens twice.
Maybe the listener should be called asynchronously? Currently I'm moving the calling `setFullZoom` call outside the listener, to avoid the problem.
 
Here the stack:

 06:20:28     INFO -  3856 INFO TEST-START | dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html
 06:20:28     INFO -  ++DOMWINDOW == 68 (0x1118af400) [pid = 2908] [serial = 68] [outer = 0x11be38000]
 06:20:28     INFO -  ++DOCSHELL 0x11ba67800 == 11 [pid = 2908] [id = 17]
 06:20:28     INFO -  ++DOMWINDOW == 69 (0x1118b3000) [pid = 2908] [serial = 69] [outer = 0x0]
 06:20:28     INFO -  ++DOMWINDOW == 70 (0x1118b4400) [pid = 2908] [serial = 70] [outer = 0x1118b3000]
 06:20:28     INFO -  [Child 2908] ###!!! ASSERTION: two zooms happening at the same time? impossible!: '!mSuppressResizeReflow', file /builds/slave/try-m64-d-00000000000000000000/build/src/layout/base/nsPresContext.cpp, line 1298
 06:21:14     INFO -  #01: nsDocumentViewer::SetFullZoom(float) [xpcom/glue/nsCOMPtr.h:746]
 06:21:14     INFO -  #02: NS_InvokeByIndex [xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:180]
 06:21:14     INFO -  #03: CallMethodHelper::Call() [js/xpconnect/src/XPCWrappedNative.cpp:1385]
 06:21:14     INFO -  #04: XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*) [js/xpconnect/src/XPCWrappedNative.cpp:1352]
 06:21:14     INFO -  #05: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [js/src/jscntxtinlines.h:236]
 06:21:14     INFO -  #06: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:441]
 06:21:14     INFO -  #07: js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) [js/src/vm/Interpreter.cpp:517]
 06:21:14     INFO -  #08: js::NativeSetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::QualifiedBool, JS::ObjectOpResult&) [js/src/vm/NativeObject.cpp:2364]
 06:21:14     INFO -  #09: js::SetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) [js/src/vm/NativeObject.h:1497]
 06:21:14     INFO -  #10: Interpret [js/src/vm/Interpreter.cpp:256]
 06:21:14     INFO -  #11: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:399]
 06:21:14     INFO -  #12: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:471]
 06:21:14     INFO -  #13: <name omitted> [js/src/vm/Interpreter.cpp:517]
 06:21:14     INFO -  #14: js::Wrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const [js/src/proxy/Wrapper.cpp:165]
 06:21:14     INFO -  #15: js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const [js/src/proxy/CrossCompartmentWrapper.cpp:335]
 06:21:14     INFO -  #16: js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) [js/src/proxy/Proxy.cpp:401]
 06:21:14     INFO -  #17: js::proxy_Call(JSContext*, unsigned int, JS::Value*) [js/public/RootingAPI.h:692]
 06:21:14     INFO -  #18: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [js/src/jscntxtinlines.h:236]
 06:21:14     INFO -  #19: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:441]
 06:21:14     INFO -  #20: Interpret [js/src/vm/Interpreter.cpp:2881]
 06:21:14     INFO -  #21: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:399]
 06:21:14     INFO -  #22: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:471]
 06:21:14     INFO -  #23: Interpret [js/src/vm/Interpreter.cpp:2881]
 06:21:14     INFO -  #24: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:399]
 06:21:14     INFO -  #25: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:471]
 06:21:14     INFO -  #26: <name omitted> [js/src/vm/Interpreter.cpp:517]
 06:21:14     INFO -  #27: JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) [js/src/jsapi.cpp:2839]
 06:21:14     INFO -  #28: mozilla::dom::MediaQueryListListener::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::MediaQueryList&, mozilla::ErrorResult&) [obj-firefox/dom/bindings/MediaQueryListBinding.cpp:36]
 06:21:14     INFO -  #29: mozilla::dom::MediaQueryListListener::Call(mozilla::dom::MediaQueryList&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) [obj-firefox/dist/include/mozilla/dom/MediaQueryListBinding.h:86]
 06:21:14     INFO -  #30: nsPresContext::MediaFeatureValuesChanged(nsRestyleHint, nsChangeHint) [dom/bindings/ErrorResult.h:535]
 06:21:14     INFO -  #31: nsPresContext::AppUnitsPerDevPixelChanged() [mfbt/RefPtr.h:306]
 06:21:14     INFO -  #32: nsPresContext::SetFullZoom(float) [layout/base/nsPresContext.cpp:1308]
 06:21:14     INFO -  #33: nsDocumentViewer::SetFullZoom(float) [xpcom/glue/nsCOMPtr.h:746]
 06:21:14     INFO -  #34: NS_InvokeByIndex [xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:180]
 06:21:14     INFO -  #35: CallMethodHelper::Call() [js/xpconnect/src/XPCWrappedNative.cpp:1385]
 06:21:14     INFO -  #36: XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*) [js/xpconnect/src/XPCWrappedNative.cpp:1352]
 06:21:14     INFO -  #37: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [js/src/jscntxtinlines.h:236]
 06:21:14     INFO -  #38: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:441]
 06:21:14     INFO -  #39: js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) [js/src/vm/Interpreter.cpp:517]
 06:21:14     INFO -  #40: js::NativeSetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::QualifiedBool, JS::ObjectOpResult&) [js/src/vm/NativeObject.cpp:2364]
 06:21:14     INFO -  #41: js::SetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) [js/src/vm/NativeObject.h:1497]
 06:21:14     INFO -  #42: Interpret [js/src/vm/Interpreter.cpp:256]
 06:21:14     INFO -  #43: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:399]
 06:21:14     INFO -  #44: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:471]
 06:21:14     INFO -  #45: <name omitted> [js/src/vm/Interpreter.cpp:517]
 06:21:14     INFO -  #46: js::Wrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const [js/src/proxy/Wrapper.cpp:165]
 06:21:14     INFO -  #47: js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const [js/src/proxy/CrossCompartmentWrapper.cpp:335]
 06:21:14     INFO -  #48: js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) [js/src/proxy/Proxy.cpp:401]
 06:21:14     INFO -  #49: js::proxy_Call(JSContext*, unsigned int, JS::Value*) [js/public/RootingAPI.h:692]
 06:21:14     INFO -  #50: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [js/src/jscntxtinlines.h:236]
 06:21:14     INFO -  #51: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:441]
 06:21:14     INFO -  #52: Interpret [js/src/vm/Interpreter.cpp:2881]
 06:21:14     INFO -  #53: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:399]
 06:21:14     INFO -  #54: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:471]
 06:21:14     INFO -  #55: Interpret [js/src/vm/Interpreter.cpp:2881]
 06:21:14     INFO -  #56: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:399]
 06:21:14     INFO -  #57: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:471]
 06:21:14     INFO -  #58: Interpret [js/src/vm/Interpreter.cpp:2881]
 06:21:14     INFO -  #59: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:399]
 06:21:14     INFO -  #60: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:471]
 06:21:14     INFO -  #61: Interpret [js/src/vm/Interpreter.cpp:2881]
 06:21:14     INFO -  #62: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:399]
 06:21:14     INFO -  #63: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [js/src/vm/Interpreter.cpp:471]
 06:21:14     INFO -  #64: <name omitted> [js/src/vm/Interpreter.cpp:517]
 06:21:14     INFO -  #65: JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) [js/src/jsapi.cpp:2780]
 06:21:14     INFO -  #66: nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) [js/xpconnect/src/XPCWrappedJSClass.cpp:1234]
 06:21:14     INFO -  #67: PrepareAndDispatch [xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_darwin.cpp:122]
 06:21:14     INFO -  MEMORY STAT | vsize 3050MB | residentFast 139MB | heapAllocated 31MB
 06:21:14     INFO -  3857 INFO TEST-OK | dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html | took 455ms
 06:21:14     INFO -  ++DOMWINDOW == 71 (0x1118b1000) [pid = 2908] [serial = 71] [outer = 0x11be38000]
 06:21:14     INFO -  3858 INFO TEST-UNEXPECTED-ERROR | dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html | Assertion count 1 is greater than expected range 0-0 assertions.
06:21:14 INFO - {u'max_asserts': 0, u'assertions': 1, u'min_asserts': 0}
dbaron, do you have an opinion on this? I can see three different approaches for fixing it:
 - the test shouldn't change fullzoom in its media query listener, or
 - media query listeners should be called asynchronously, or
 - nsPresContext::SetFullZoom should have an nsAutoScriptBlocker
Flags: needinfo?(dbaron)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.