Closed Bug 1239463 Opened 4 years ago Closed 4 years ago

Intermittent test_crashing.html | Assertion count 1 is greater than expected range 0-0 assertions.

Categories

(Core :: Plug-ins, defect)

Product:
Core
Component:
Plug-ins
Version:
46 Branch
Type:
defect
Priority:
Not set

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla46
Tracking Flags:
Tracking Status
firefox46 --- fixed

People

(Reporter: KWierso, Assigned: jdm)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Do not assert when notifying an inactive document about changed content from the plugin crash notification
1.54 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review 
Reported assertion seems to be:
 13:14:43     INFO -  [Child 12968] ###!!! ASSERTION: Something is confused: 'InActiveDocument(thisContent)', file /builds/slave/m-cen-lx-d-0000000000000000000/build/src/dom/base/nsObjectLoadingContent.cpp, line 2700
 13:14:43     INFO -  #01: nsObjectLoadingContent::NotifyStateChanged(nsObjectLoadingContent::ObjectType, mozilla::EventStates, bool, bool) [dom/base/nsObjectLoadingContent.cpp:2700]
 13:14:43     INFO -  #02: nsObjectLoadingContent::LoadFallback(nsObjectLoadingContent::FallbackType, bool) [dom/base/nsObjectLoadingContent.cpp:3034]
 13:14:43     INFO -  #03: nsObjectLoadingContent::PluginCrashed(nsIPluginTag*, nsAString_internal const&, nsAString_internal const&, bool) [dom/base/nsObjectLoadingContent.cpp:2845]
 13:14:43     INFO -  #04: nsPluginHost::PluginCrashed(nsNPAPIPlugin*, nsAString_internal const&, nsAString_internal const&) [dom/plugins/base/nsPluginHost.cpp:4052]
 13:14:43     INFO -  #05: nsNPAPIPlugin::PluginCrashed(nsAString_internal const&, nsAString_internal const&) [dom/plugins/base/nsNPAPIPlugin.cpp:252]
 13:14:43     INFO -  #06: mozilla::plugins::PluginModuleParent::NotifyPluginCrashed() [dom/plugins/ipc/PluginModuleParent.cpp:1680]
 13:14:43     INFO -  #07: mozilla::plugins::TaskFactory<mozilla::plugins::PluginModuleParent>::RunnableMethod<void (mozilla::plugins::PluginModuleParent::*)(), Tuple0>::Run() [ipc/chromium/src/base/tuple.h:387]
 13:14:43     INFO -  #08: MessageLoop::RunTask(Task*) [ipc/chromium/src/base/message_loop.cc:365]
 13:14:43     INFO -  #09: MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) [ipc/chromium/src/base/message_loop.cc:375]
 13:14:43     INFO -  #10: MessageLoop::DoWork() [ipc/chromium/src/base/message_loop.cc:459]
 13:14:43     INFO -  #11: mozilla::ipc::DoWorkRunnable::Run() [ipc/glue/MessagePump.cpp:221]
 13:14:43     INFO -  #12: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:989]
 13:14:43     INFO -  #13: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/glue/nsThreadUtils.cpp:297]
 13:14:43     INFO -  #14: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:96]
 13:14:43     INFO -  #15: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:234]
 13:14:43     INFO -  #16: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:520]
 13:14:43     INFO -  #17: nsBaseAppShell::Run() [widget/nsBaseAppShell.cpp:158]
 13:14:43     INFO -  #18: XRE_RunAppShell [toolkit/xre/nsEmbedFunctions.cpp:789]
 13:14:43     INFO -  #19: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:259]
 13:14:43     INFO -  #20: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:234]
 13:14:43     INFO -  #21: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:520]
 13:14:43     INFO -  #22: XRE_InitChildProcess [toolkit/xre/nsEmbedFunctions.cpp:629]
 13:14:43     INFO -  #23: content_process_main(int, char**) [ipc/contentproc/plugin-container.cpp:238]
The assertion in nsObjectLoadingContent::NotifyStateChanged was last modified in bug 764480, but I suspect that this is a similar issue as the one described there. In short, the test reloads the iframe that contains the plugin in the same JS execution frame as the call to plugin.crash(). The plugin crash notification will be queued in the event loop, and conceivably some of the reloading steps could execute either before or after it is processed, causing the intermittent behaviour seen here. I suspect the solution here is to either turn the assertion into an early return, or remove it. This isn't a synchronous notification, so FlushPendingNotifications won't be called. That means that there's a script blocker and the document's ContentStateChanged is called, but I'm not sure what the implications of that are.

Boris, is this anything that you're familiar with?
Flags: needinfo?(bzbarsky)
Calling ContentStateChanged is not a problem for an inactive document.  It can totally happen for other reasons.

I can't speak to why the assertion is there.
Flags: needinfo?(bzbarsky)
The original assertion is from pre-hg days. The only modification was specifically making it check whether the document is active or not in bug 764480. I'm inclined to only assert if aSync is true, when FlushPendingNotifications would be called. Does that sound sensible, Boris?
Flags: needinfo?(bzbarsky)
That seems fine, yes.

But note that bug 764480 made the assertion _stronger_.  Before that it only asserted we're in doc; now it asserts we're in an active doc....  That last part is what's presumably failing here.
Flags: needinfo?(bzbarsky)
Assignee: nobody → josh
Status: NEW → ASSIGNED
Comment on attachment 8710578 [details] [diff] [review]
Do not assert when notifying an inactive document about changed content from the plugin crash notification

r=me
Attachment #8710578 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7a439abc3b9b1bbf3b2da7e7ccd17c360f7eedc
Bug 1239463 - Do not assert when notifying an inactive document about changed content from the plugin crash notification. r=bz
https://hg.mozilla.org/mozilla-central/rev/e7a439abc3b9
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.