Tyson Smith [:tsmith] Reporter
	Description • 7 years ago

This is triggered when loading http://acid3.acidtests.org/ when built with "-fsanitize=float-divide-by-zero,integer-divide-by-zero"

/mozilla-central/dom/html/ImageDocument.cpp:433:61: runtime error: division by zero
    #0 0x7f8ec70c2183 in mozilla::dom::ImageDocument::ScrollImageTo(int, int, bool) /mozilla-central/dom/html/ImageDocument.cpp:433:61
    #1 0x7f8ec70c180c in mozilla::dom::ImageDocument::ShrinkToFit() /mozilla-central/dom/html/ImageDocument.cpp:384:3
    #2 0x7f8ec70c59ea in mozilla::dom::ImageDocument::CheckOverflowing(bool) /mozilla-central/dom/html/ImageDocument.cpp:751:7
    #3 0x7f8ec70e7f7e in applyImpl<mozilla::dom::ImageDocument, void (mozilla::dom::ImageDocument::*)()> /mozilla-central/objdir-ff-ubsan/dist/include/nsThreadUtils.h:1142:12
    #4 0x7f8ec70e7f7e in apply<mozilla::dom::ImageDocument, void (mozilla::dom::ImageDocument::*)()> /mozilla-central/objdir-ff-ubsan/dist/include/nsThreadUtils.h:1148
    #5 0x7f8ec70e7f7e in mozilla::detail::RunnableMethodImpl<mozilla::dom::ImageDocument*, void (mozilla::dom::ImageDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /mozilla-central/objdir-ff-ubsan/dist/include/nsThreadUtils.h:1192
    #6 0x7f8ec4b7e3d4 in nsContentUtils::RemoveScriptBlocker() /mozilla-central/dom/base/nsContentUtils.cpp:5735:15
    #7 0x7f8ec4c9baa7 in ~nsAutoScriptBlocker /mozilla-central/dom/base/nsContentUtils.h:3516:5
    #8 0x7f8ec4c9baa7 in nsImageLoadingContent::Notify(imgIRequest*, int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*) /mozilla-central/dom/base/nsImageLoadingContent.cpp:172
    #9 0x7f8ec49f2310 in imgRequestProxy::Notify(int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*) /mozilla-central/image/imgRequestProxy.cpp:967:13
    #10 0x7f8ec4a2a0b9 in operator() /mozilla-central/image/ProgressTracker.cpp:333:48
    #11 0x7f8ec4a2a0b9 in void mozilla::image::ImageObserverNotifier<mozilla::image::ObserverTable const*>::operator()<void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)::{lambda(mozilla::image::IProgressObserver*)#1}>(void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)::{lambda(mozilla::image::IProgressObserver*)#1}) /mozilla-central/image/ProgressTracker.cpp:292
    #12 0x7f8ec4a29c96 in void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /mozilla-central/image/ProgressTracker.cpp:333:5
    #13 0x7f8ec499c802 in operator() /mozilla-central/image/ProgressTracker.cpp:412:5
    #14 0x7f8ec499c802 in Read<(lambda at /mozilla-central/image/ProgressTracker.cpp:411:19)> /mozilla-central/image/CopyOnWrite.h:154
    #15 0x7f8ec499c802 in mozilla::image::ProgressTracker::SyncNotifyProgress(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /mozilla-central/image/ProgressTracker.cpp:411
    #16 0x7f8ec49a61cd in mozilla::image::RasterImage::NotifyProgress(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::Maybe<unsigned int> const&, mozilla::image::DecoderFlags, mozilla::image::SurfaceFlags) /mozilla-central/image/RasterImage.cpp:1757:28
    #17 0x7f8ec49b2f8a in mozilla::image::RasterImage::NotifyDecodeComplete(mozilla::image::DecoderFinalStatus const&, mozilla::image::ImageMetadata const&, mozilla::image::DecoderTelemetry const&, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::Maybe<unsigned int> const&, mozilla::image::DecoderFlags, mozilla::image::SurfaceFlags) /mozilla-central/image/RasterImage.cpp:1797:3
    #18 0x7f8ec4983f15 in operator() /mozilla-central/image/IDecodingTask.cpp:130:12
    #19 0x7f8ec4983f15 in mozilla::detail::RunnableFunction<mozilla::image::IDecodingTask::NotifyDecodeComplete(mozilla::NotNull<mozilla::image::RasterImage*>, mozilla::NotNull<mozilla::image::Decoder*>)::$_2>::Run() /mozilla-central/xpcom/threads/nsThreadUtils.h:529
    #20 0x7f8ec170ef47 in mozilla::SchedulerGroup::Runnable::Run() /mozilla-central/xpcom/threads/SchedulerGroup.cpp:396:25
    #21 0x7f8ec1742f7f in nsThread::ProcessNextEvent(bool, bool*) /mozilla-central/xpcom/threads/nsThread.cpp:1037:14
    #22 0x7f8ec1774b09 in NS_ProcessNextEvent(nsIThread*, bool) /mozilla-central/xpcom/threads/nsThreadUtils.cpp:513:10
    #23 0x7f8ec2902468 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /mozilla-central/ipc/glue/MessagePump.cpp:97:21
    #24 0x7f8ec2790589 in RunHandler /mozilla-central/ipc/chromium/src/base/message_loop.cc:319:3
    #25 0x7f8ec2790589 in MessageLoop::Run() /mozilla-central/ipc/chromium/src/base/message_loop.cc:299
    #26 0x7f8ec8707164 in nsBaseAppShell::Run() /mozilla-central/widget/nsBaseAppShell.cpp:158:27
    #27 0x7f8ecd81eef8 in XRE_RunAppShell() /mozilla-central/toolkit/xre/nsEmbedFunctions.cpp:877:22
    #28 0x7f8ec2790589 in RunHandler /mozilla-central/ipc/chromium/src/base/message_loop.cc:319:3
    #29 0x7f8ec2790589 in MessageLoop::Run() /mozilla-central/ipc/chromium/src/base/message_loop.cc:299
    #30 0x7f8ecd81e4ce in XRE_InitChildProcess(int, char**, XREChildData const*) /mozilla-central/toolkit/xre/nsEmbedFunctions.cpp:703:34
    #31 0x5171d8 in content_process_main /mozilla-central/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
    #32 0x5171d8 in main /mozilla-central/browser/app/nsBrowserApp.cpp:280
    #33 0x7f8ef07931c0 in __libc_start_main /build/glibc-CxtIbX/glibc-2.26/csu/../csu/libc-start.c:308
    #34 0x41f789 in _start (/mozilla-central/objdir-ff-ubsan/dist/bin/firefox+0x41f789)

	Andrew Overholt [:overholt]
	Comment 1 • 7 years ago

Seems to be related to code Jonathan wrote but maybe the issue is coming from some caller in the stack?

	Jonathan Kew [:jfkthame]
	Comment 2 • 7 years ago

I plead not guilty here. :)

It looks like /ratio/ must be zero here, to get a divide-by-zero error on the line in question. Blame says I touched that line in b85353b6cc38, but that was just a backout of cad945d3a7b1. Prior to that, the same code was present in http://searchfox.org/mozilla-central/rev/027d6f3063f512cb0790c450e975d797e3a30938/content/html/document/src/ImageDocument.cpp#466.

The root of the problem seems to be that GetRatio() is returning zero, and we don't expect that.

It looks like that would occur any time mVisibleWidth or mVisibleHeight is zero. So we should probably be checking for that, and taking an early return somewhere, or whatever...

	Hsin-Yi Tsai (she/her) (REO Fx 129) [:hsinyi]
	Comment 3 • 7 years ago

Edgar has been working on image element and probably has thoughts.

	Edgar Chen [:edgar] (Parental leave until Oct. 09.) Assignee
	Comment 4 • 7 years ago

(In reply to Jonathan Kew (:jfkthame) from comment #2) 
> It looks like that would occur any time mVisibleWidth or mVisibleHeight is
> zero.

Yeah, in this case, the ImageDocument is in a hidden iframe, mVisibleWidth and mVisibleHeight are both zero.
It looks like we don't need to resize image if the document is not visible.

	Edgar Chen [:edgar] (Parental leave until Oct. 09.) Assignee
	Comment 6 • 7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=18fa6acc532742f050a1788680c89761f1eb312a&filter-tier=1&group_state=expanded

	Edgar Chen [:edgar] (Parental leave until Oct. 09.) Assignee
	Comment 7 • 7 years ago

(In reply to Edgar Chen [:edgar] from comment #4)
> (In reply to Jonathan Kew (:jfkthame) from comment #2) 
> > It looks like that would occur any time mVisibleWidth or mVisibleHeight is
> > zero.
> 
> Yeah, in this case, the ImageDocument is in a hidden iframe, mVisibleWidth
> and mVisibleHeight are both zero.
> It looks like we don't need to resize image if the document is not visible.

But if the image could be requested to resize from shrinkToFit() calls, doing nothing in this case seems weird, unless we have a way to inform the caller what is happened, IMO. So it looks like we should still do resize, but don't try to scroll image if the document is not visible.

	Boris Zbarsky [:bzbarsky]
	Comment 9 • 7 years ago
mozreview-review

Comment on attachment 8925859 [details]
Bug 1414077 - Don't try to scroll image if the image document is not visible;

https://reviewboard.mozilla.org/r/197058/#review204600

r=me.  Thank you!

	Pulsebot
	Comment 10 • 7 years ago

Pushed by echen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efbe44c7fa03
Don't try to scroll image if the image document is not visible; r=bz

	Noemi Erli[:noemi_erli]
	Comment 11 • 7 years ago
bugherder

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

	Gerry Chang [:gchang]
	Comment 12 • 7 years ago

Won't fix for 58. Let it ride the train.