Closed Bug 1265611 Opened 3 years ago Closed 3 years ago

layout/style/crashtests/696869-1.html is going to permafail when Gecko 48 merges to Beta (Assertion failure: mProperties.Length() == 1 (Transitions should have exactly one animation property. Perhaps we are using an un-initialized transition?))

Categories

(Core :: DOM: Animation, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox46 --- unaffected
firefox47 + fixed
firefox48 + fixed

People

(Reporter: RyanVM, Assigned: birtles)

References

Details

(Keywords: assertion, crash)

Attachments

(5 files)

[Tracking Requested - why for this release]: Permafail when Gecko 48 merges to Beta.

https://treeherder.mozilla.org/logviewer.html#?job_id=19616828&repo=try

08:47:26     INFO -  REFTEST TEST-START | file:///builds/slave/test/build/tests/reftest/tests/layout/style/crashtests/696869-1.html
08:47:26     INFO -  REFTEST TEST-LOAD | file:///builds/slave/test/build/tests/reftest/tests/layout/style/crashtests/696869-1.html | 2381 / 3025 (78%)
08:47:26     INFO -  ++DOMWINDOW == 121 (0x95c21c00) [pid = 1944] [serial = 5620] [outer = 0x99b85400]
08:47:26     INFO -  Assertion failure: mProperties.Length() == 1 (Transitions should have exactly one animation property. Perhaps we are using an un-initialized transition?), at /builds/slave/try-lx-d-000000000000000000000/build/src/layout/style/nsTransitionManager.h:58
08:47:26     INFO -  #01: mozilla::ElementPropertyTransition::TransitionProperty [layout/style/nsTransitionManager.h:56]
08:47:26     INFO -  #02: nsTransitionManager::ConsiderStartingTransition [layout/style/nsTransitionManager.cpp:580]
08:47:26     INFO -  #03: nsTransitionManager::UpdateTransitions [layout/style/nsTransitionManager.cpp:418]
08:47:26     INFO -  #04: nsTransitionManager::StyleContextChanged [layout/style/nsTransitionManager.cpp:352]
08:47:26     INFO -  #05: mozilla::RestyleManager::TryStartingTransition [mfbt/RefPtr.h:503]
08:47:26     INFO -  #06: mozilla::ElementRestyler::RestyleSelf [layout/base/RestyleManager.cpp:4105]
08:47:26     INFO -  #07: mozilla::ElementRestyler::Restyle [layout/base/RestyleManager.cpp:3291]
08:47:26     INFO -  #08: mozilla::ElementRestyler::ComputeStyleChangeFor [layout/base/RestyleManager.cpp:4529]
08:47:26     INFO -  #09: mozilla::RestyleManager::ComputeAndProcessStyleChange [layout/base/RestyleManager.cpp:4941]
08:47:26     INFO -  #10: mozilla::RestyleManager::RestyleElement [layout/base/RestyleManager.cpp:1068]
08:47:26     INFO -  #11: mozilla::RestyleTracker::ProcessOneRestyle [layout/base/RestyleTracker.cpp:95]
08:47:26     INFO -  #12: mozilla::RestyleTracker::DoProcessRestyles [layout/base/RestyleTracker.cpp:264]
08:47:26     INFO -  #13: mozilla::RestyleManager::ProcessPendingRestyles [layout/base/RestyleManager.cpp:1786]
08:47:26     INFO -  #14: PresShell::FlushPendingNotifications [layout/base/nsPresShell.cpp:4064]
08:47:26     INFO -  #15: nsRefreshDriver::Tick [mfbt/RefPtr.h:261]
08:47:26     INFO -  #16: mozilla::RefreshDriverTimer::TickRefreshDrivers [layout/base/nsRefreshDriver.cpp:246]
08:47:26     INFO -  #17: mozilla::RefreshDriverTimer::Tick [layout/base/nsRefreshDriver.cpp:265]
08:47:26     INFO -  #18: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver [layout/base/nsRefreshDriver.cpp:588]
08:47:26     INFO -  #19: nsRunnableMethodImpl<void (mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)(mozilla::TimeStamp), true, mozilla::TimeStamp>::Run [xpcom/glue/nsThreadUtils.h:671]
08:47:26     INFO -  #20: nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:994]
08:47:26     INFO -  #21: NS_ProcessNextEvent [xpcom/glue/nsThreadUtils.cpp:290]
08:47:26     INFO -  #22: mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:99]
08:47:26     INFO -  #23: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:230]
08:47:26     INFO -  #24: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:505]
08:47:26     INFO -  #25: nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:158]
08:47:26     INFO -  #26: nsAppStartup::Run [toolkit/components/startup/nsAppStartup.cpp:282]
08:47:26     INFO -  #27: XREMain::XRE_mainRun [toolkit/xre/nsAppRunner.cpp:4345]
08:47:26     INFO -  #28: XREMain::XRE_main [toolkit/xre/nsAppRunner.cpp:4441]
08:47:26     INFO -  #29: XRE_main [toolkit/xre/nsAppRunner.cpp:4548]
08:47:26     INFO -  #30: do_main [browser/app/nsBrowserApp.cpp:220]
08:47:26     INFO -  #31: main [browser/app/nsBrowserApp.cpp:360]
08:47:26     INFO -  ExceptionHandler::GenerateDump cloned child 2212
08:47:26     INFO -  ExceptionHandler::SendContinueSignalToChild sent continue signal to child
08:47:26     INFO -  ExceptionHandler::WaitForContinueSignal waiting for continue signal...
08:47:27  WARNING -  TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/style/crashtests/696869-1.html | application terminated with exit code 11
08:47:27     INFO -  REFTEST INFO | Copy/paste: /builds/slave/test/build/linux32-minidump_stackwalk /tmp/tmp4rb5eg.mozrunner/minidumps/1e7f6a03-96a0-61e0-60328f36-11b6a0d9.dmp /builds/slave/test/build/symbols
08:47:43     INFO -  REFTEST INFO | Saved minidump as /builds/slave/test/build/blobber_upload_dir/1e7f6a03-96a0-61e0-60328f36-11b6a0d9.dmp
08:47:43     INFO -  REFTEST INFO | Saved app info as /builds/slave/test/build/blobber_upload_dir/1e7f6a03-96a0-61e0-60328f36-11b6a0d9.extra
08:47:43    ERROR -  REFTEST PROCESS-CRASH | file:///builds/slave/test/build/tests/reftest/tests/layout/style/crashtests/696869-1.html | application crashed [@ mozilla::ElementPropertyTransition::TransitionProperty]
08:47:43     INFO -  Crash dump filename: /tmp/tmp4rb5eg.mozrunner/minidumps/1e7f6a03-96a0-61e0-60328f36-11b6a0d9.dmp
08:47:43     INFO -  Operating system: Linux
08:47:43     INFO -                    0.0.0 Linux 3.2.0-76-generic-pae #111-Ubuntu SMP Tue Jan 13 22:34:29 UTC 2015 i686
08:47:43     INFO -  CPU: x86
08:47:43     INFO -       GenuineIntel family 6 model 45 stepping 7
08:47:43     INFO -       1 CPU
08:47:43     INFO -  Crash reason:  SIGSEGV
08:47:43     INFO -  Crash address: 0x0
08:47:43     INFO -  Process uptime: not available
08:47:43     INFO -  Thread 0 (crashed)
08:47:43     INFO -   0  libxul.so!mozilla::ElementPropertyTransition::TransitionProperty [nsTransitionManager.h:3bfd2ef48fd3 : 56 + 0x0]
08:47:43     INFO -      eip = 0xb33e9071   esp = 0xbff544c0   ebp = 0xbff544d8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0x70ffc0a0   edi = 0x55504bc4   eax = 0xb4e25690   ecx = 0xb60e5654
08:47:43     INFO -      edx = 0x00000000   efl = 0x00210282
08:47:43     INFO -      Found by: given as instruction pointer in context
08:47:43     INFO -   1  libxul.so!nsTransitionManager::ConsiderStartingTransition [nsTransitionManager.cpp:3bfd2ef48fd3 : 580 + 0x5]
08:47:43     INFO -      eip = 0xb33f9bab   esp = 0xbff544e0   ebp = 0xbff54638   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0x70ffc0a0   edi = 0x55504bc4
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -   2  libxul.so!nsTransitionManager::UpdateTransitions [nsTransitionManager.cpp:3bfd2ef48fd3 : 423 + 0x8]
08:47:43     INFO -      eip = 0xb33fa94d   esp = 0xbff54640   ebp = 0xbff54718   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xa2bc4674   edi = 0x0000010b
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -   3  libxul.so!nsTransitionManager::StyleContextChanged [nsTransitionManager.cpp:3bfd2ef48fd3 : 352 + 0x18]
08:47:43     INFO -      eip = 0xb3420126   esp = 0xbff54720   ebp = 0xbff547c8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xa1ff4550   edi = 0xa6e62be0
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -   4  libxul.so!mozilla::RestyleManager::TryStartingTransition [RestyleManager.cpp:3bfd2ef48fd3 : 2136 + 0x1c]
08:47:43     INFO -      eip = 0xb343360d   esp = 0xbff547d0   ebp = 0xbff54808   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xbff5486c   edi = 0x6aab7d60
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -   5  libxul.so!mozilla::ElementRestyler::RestyleSelf [RestyleManager.cpp:3bfd2ef48fd3 : 4104 + 0x1d]
08:47:43     INFO -      eip = 0xb344d47c   esp = 0xbff54810   ebp = 0xbff54908   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0x6aab7d60   edi = 0xbff5489c
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -   6  libxul.so!mozilla::ElementRestyler::Restyle [RestyleManager.cpp:3bfd2ef48fd3 : 3289 + 0x17]
08:47:43     INFO -      eip = 0xb344c3bf   esp = 0xbff54910   ebp = 0xbff549c8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xbff54a68   edi = 0xa1ff5bd0
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -   7  libxul.so!mozilla::ElementRestyler::ComputeStyleChangeFor [RestyleManager.cpp:3bfd2ef48fd3 : 4527 + 0x12]
08:47:43     INFO -      eip = 0xb344eeee   esp = 0xbff549d0   ebp = 0xbff54cd8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xa1ff5bd0   edi = 0x00000000
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -   8  libxul.so!mozilla::RestyleManager::ComputeAndProcessStyleChange [RestyleManager.cpp:3bfd2ef48fd3 : 4940 + 0x1c]
08:47:43     INFO -      eip = 0xb344f58b   esp = 0xbff54ce0   ebp = 0xbff54dc8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xbff54d24   edi = 0xbff54d28
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -   9  libxul.so!mozilla::RestyleManager::RestyleElement [RestyleManager.cpp:3bfd2ef48fd3 : 1068 + 0x15]
08:47:43     INFO -      eip = 0xb344f92f   esp = 0xbff54dd0   ebp = 0xbff54e28   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0x95c9fc00   edi = 0xa1ff5bd0
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  10  libxul.so!mozilla::RestyleTracker::ProcessOneRestyle [RestyleTracker.cpp:3bfd2ef48fd3 : 95 + 0x1c]
08:47:43     INFO -      eip = 0xb3464ef9   esp = 0xbff54e30   ebp = 0xbff54f18   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0x95c9fc38   edi = 0x95ea2800
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  11  libxul.so!mozilla::RestyleTracker::DoProcessRestyles [RestyleTracker.cpp:3bfd2ef48fd3 : 263 + 0x1a]
08:47:43     INFO -      eip = 0xb34823d1   esp = 0xbff54f20   ebp = 0xbff55b58   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xbff54f78   edi = 0x55ff3368
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  12  libxul.so!mozilla::RestyleManager::ProcessPendingRestyles [RestyleManager.cpp:3bfd2ef48fd3 : 1784 + 0xc]
08:47:43     INFO -      eip = 0xb344a152   esp = 0xbff55b60   ebp = 0xbff55b88   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0x95c9fc00   edi = 0x715b8408
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  13  libxul.so!PresShell::FlushPendingNotifications [nsPresShell.cpp:3bfd2ef48fd3 : 4064 + 0x5]
08:47:43     INFO -      eip = 0xb34f398d   esp = 0xbff55b90   ebp = 0xbff55c78   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0x715b8400   edi = 0x715b8408
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  14  libxul.so!nsRefreshDriver::Tick [nsRefreshDriver.cpp:3bfd2ef48fd3 : 1739 + 0x13]
08:47:43     INFO -      eip = 0xb342a387   esp = 0xbff55c80   ebp = 0xbff55dc8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xbff55d64   edi = 0x95bdcb60
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  15  libxul.so!mozilla::RefreshDriverTimer::TickRefreshDrivers [nsRefreshDriver.cpp:3bfd2ef48fd3 : 246 + 0x15]
08:47:43     INFO -      eip = 0xb342b4d2   esp = 0xbff55dd0   ebp = 0xbff55e18   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0x2a3b144c   edi = 0x2a3b1448
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  16  libxul.so!mozilla::RefreshDriverTimer::Tick [nsRefreshDriver.cpp:3bfd2ef48fd3 : 264 + 0x13]
08:47:43     INFO -      eip = 0xb342b5aa   esp = 0xbff55e20   ebp = 0xbff55e58   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xa0970a60   edi = 0xb61174c4
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  17  libxul.so!mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver [nsRefreshDriver.cpp:3bfd2ef48fd3 : 588 + 0x8]
08:47:43     INFO -      eip = 0xb342b7ed   esp = 0xbff55e60   ebp = 0xbff55ed8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xbff55eb0   edi = 0xa0970a60
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  18  libxul.so!nsRunnableMethodImpl<void (mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)(mozilla::TimeStamp), true, mozilla::TimeStamp>::Run [nsThreadUtils.h:3bfd2ef48fd3 : 671 + 0x1d]
08:47:43     INFO -      eip = 0xb342308c   esp = 0xbff55ee0   ebp = 0xbff55ef8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0x00000000   edi = 0xb60e9c08
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  19  libxul.so!nsThread::ProcessNextEvent [nsThread.cpp:3bfd2ef48fd3 : 994 + 0x14]
08:47:43     INFO -      eip = 0xb16d8dbf   esp = 0xbff55f00   ebp = 0xbff55f78   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xb7137c00   edi = 0xb60e9c08
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  20  libxul.so!NS_ProcessNextEvent [nsThreadUtils.cpp:3bfd2ef48fd3 : 290 + 0x10]
08:47:43     INFO -      eip = 0xb16ff525   esp = 0xbff55f80   ebp = 0xbff55fb8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xafd0eb20   edi = 0xb71267c0
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  21  libxul.so!mozilla::ipc::MessagePump::Run [MessagePump.cpp:3bfd2ef48fd3 : 98 + 0xc]
08:47:43     INFO -      eip = 0xb1a87e37   esp = 0xbff55fc0   ebp = 0xbff56008   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xafd0eb20   edi = 0xb71267c0
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  22  libxul.so!MessageLoop::RunInternal [message_loop.cc:3bfd2ef48fd3 : 230 + 0x14]
08:47:43     INFO -      eip = 0xb1a641ac   esp = 0xbff56010   ebp = 0xbff56038   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xb71267c0   edi = 0xb7137c00
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  23  libxul.so!MessageLoop::Run [message_loop.cc:3bfd2ef48fd3 : 223 + 0x8]
08:47:43     INFO -      eip = 0xb1a641d2   esp = 0xbff56040   ebp = 0xbff56068   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xb71267c0   edi = 0xb7137c00
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  24  libxul.so!nsBaseAppShell::Run [nsBaseAppShell.cpp:3bfd2ef48fd3 : 156 + 0xe]
08:47:43     INFO -      eip = 0xb32373c9   esp = 0xbff56070   ebp = 0xbff56098   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xa9ce03d0   edi = 0xb7137c00
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  25  libxul.so!nsAppStartup::Run [nsAppStartup.cpp:3bfd2ef48fd3 : 281 + 0x9]
08:47:43     INFO -      eip = 0xb3a6b8ef   esp = 0xbff560a0   ebp = 0xbff560b8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xac245460   edi = 0xbff56339
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  26  libxul.so!XREMain::XRE_mainRun [nsAppRunner.cpp:3bfd2ef48fd3 : 4344 + 0x17]
08:47:43     INFO -      eip = 0xb3abbcba   esp = 0xbff560c0   ebp = 0xbff561b8   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0x00000000   edi = 0xbff56339
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  27  libxul.so!XREMain::XRE_main [nsAppRunner.cpp:3bfd2ef48fd3 : 4441 + 0x9]
08:47:43     INFO -      eip = 0xb3abc0e2   esp = 0xbff561c0   ebp = 0xbff56208   ebx = 0xb60e5654
08:47:43     INFO -      esi = 0xbff56240   edi = 0xbff5625c
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  28  libxul.so!XRE_main [nsAppRunner.cpp:3bfd2ef48fd3 : 4547 + 0xf]
08:47:43     INFO -      eip = 0xb3abc322   esp = 0xbff56210   ebp = 0xbff56348   ebx = 0x0807151c
08:47:43     INFO -      esi = 0xbff56240   edi = 0xb7124700
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  29  firefox!do_main [nsBrowserApp.cpp:3bfd2ef48fd3 : 220 + 0x14]
08:47:43     INFO -      eip = 0x0804d36b   esp = 0xbff56350   ebp = 0xbff573a8   ebx = 0x0807151c
08:47:43     INFO -      esi = 0xbff57544   edi = 0xb7124700
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  30  firefox!main [nsBrowserApp.cpp:3bfd2ef48fd3 : 360 + 0x1b]
08:47:43     INFO -      eip = 0x0804c79d   esp = 0xbff573b0   ebp = 0xbff57498   ebx = 0x0807151c
08:47:43     INFO -      esi = 0xbff57544   edi = 0x00000061
08:47:43     INFO -      Found by: call frame info
08:47:43     INFO -  31  libc-2.15.so + 0x194d3
08:47:43     INFO -      eip = 0xb741a4d3   esp = 0xbff574a0   ebp = 0x00000000
08:47:43     INFO -      Found by: previous frame's frame pointer
08:47:43     INFO -  32  firefox!__libc_csu_fini + 0x10
08:47:43     INFO -      eip = 0x08066d70   esp = 0xbff574a4   ebp = 0x00000000
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  33  libc-2.15.so + 0x194d3
08:47:43     INFO -      eip = 0xb741a4d3   esp = 0xbff574b0   ebp = 0x00000000
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  34  libc-2.15.so + 0x1a4ff4
08:47:43     INFO -      eip = 0xb75a5ff4   esp = 0xbff574d8   ebp = 0x00000000
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  35  firefox + 0x4994
08:47:43     INFO -      eip = 0x0804c994   esp = 0xbff57500   ebp = 0x00000000
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  36  ld-2.15.so + 0x146b0
08:47:43     INFO -      eip = 0xb772f6b0   esp = 0xbff57508   ebp = 0x00000000
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  37  libc-2.15.so + 0x193e9
08:47:43     INFO -      eip = 0xb741a3e9   esp = 0xbff5750c   ebp = 0x00000000
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  38  ld-2.15.so + 0x20ff4
08:47:43     INFO -      eip = 0xb773bff4   esp = 0xbff57510   ebp = 0x00000000
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  39  firefox + 0x4994
08:47:43     INFO -      eip = 0x0804c994   esp = 0xbff57518   ebp = 0x00000000
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  40  firefox!_start + 0x21
08:47:43     INFO -      eip = 0x0804c9b5   esp = 0xbff57520   ebp = 0x00000000
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  41  firefox!init [replace_malloc.c:3bfd2ef48fd3 : 133 + 0x5]
08:47:43     INFO -      eip = 0x0804c6a4   esp = 0xbff57524   ebp = 0x00000000
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  42  firefox!__libc_csu_fini + 0x10
08:47:43     INFO -      eip = 0x08066d70   esp = 0xbff57530   ebp = 0xbff57544
08:47:43     INFO -      Found by: stack scanning
08:47:43     INFO -  43  0xbff58a51
08:47:43     INFO -      eip = 0xbff58a51   esp = 0xbff5754c   ebp = 0xbff58a1c
08:47:43     INFO -      Found by: previous frame's frame pointer
08:47:43     INFO -  44  0x2f73646c
08:47:43     INFO -      eip = 0x2f73646c   esp = 0xbff58a24   ebp = 0x6975622f
08:47:43     INFO -      Found by: previous frame's frame pointer
Flags: needinfo?(bbirtles)
I actually wonder if this has nothing to do with preferences and is actually related to bug 1264863?
Seems quite likely this is related to bug 1264863 so I'm taking both of them.
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Flags: needinfo?(bbirtles)
So it turns out that what happens is we end up creating a transition on -webkit-text-fill-color but then failing to fill in the properties because layout.css.prefixes.webkit is false on release builds.

So we need to do a few things here:

1. Try to create a test that reproduces (probably by forcefully turning off layout.css.prefixes.webkit)
2. Make the various TransitionProperty() and ToValue() convenience methods in nsTransitionManager work off mFrames
3. Try to test if a property is enabled before creating transitions for it.
(4. Try to test if a property is enabled when setting up frames through Element.animate() etc.)
(5. Try to test if a property is enabled when creating animations.)
I have verified that without the fix in the first patch in this series this
test fails, but passes with the fix applied.

Review commit: https://reviewboard.mozilla.org/r/47933/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47933/
Currently if we have transition-property: 'all' and trigger a transition
on the 'color' property we will end up generating a transition on
-webkit-text-fill-color even if that property is disabled.

However, when we later call StyleAnimationValue::ToValue() in
nsTransitionManager::UpdateTransitions() to see if there are any transitions we
need to cancel, the comparison for currentValue != anim->ToValue() will pass
(since, as of the first patch in this patch series, ToValue() returns a null
value) so we end up cancelling the transition as soon as we create it).

Nevertheless, we will still trigger the warning introduced in the first patch
in this series when we call ToValue().

This patch stops us from creating transitions in the first place (and hence
triggering the warning). It also removes the code that suppresses transition
events for transitions on disabled properties since we should no longer be
generating such transitions in the first place (unless the pref is switched
while the transition is in motion which is probably not worth worrying about).

Note that we only test if the property is enabled for all content. This is
consistent with what we do throughout animation code including the existing
code in nsTransitionManager which iterates through shorthand sub-properties
using CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES with the flag
nsCSSProps::eEnabledForAllContent.

The test case in this patch doesn't actually fail without this change, all it
does it trigger the warning in StyleAnimationValue::ToValue() introduced
in the first patch in this series. It's still a useful regression test however,
particularly if we later upgrade the warning in StyleAnimationValue::ToValue()
to a fatal assertion.

Review commit: https://reviewboard.mozilla.org/r/47935/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47935/
Attachment #8743617 - Flags: review?(bugzilla)
Comment on attachment 8743617 [details]
MozReview Request: Bug 1265611 - Don't trigger transitions for properties that are disabled

https://reviewboard.mozilla.org/r/47935/#review44675

::: layout/style/nsCSSProps.h:229
(Diff revision 1)
> +// Furthermore, for the purposes of animation (including triggering
> +// transitions) these flags are ignored.

Probably you should say explicitly that, transition would not be applied to any property which is not available to the content.

::: layout/style/nsCSSProps.h:229
(Diff revision 1)
> +// Furthermore, for the purposes of animation (including triggering
> +// transitions) these flags are ignored.

Is animation affected here? The code change below doesn't imply this to me. It's probably worth trying using "all" property in @keyframes and see what would happen.

::: layout/style/test/test_transitions_with_disabled_properties.html:28
(Diff revision 1)
> +<div id="display"></div>
> +<pre id="test">
> +<script>
> +SimpleTest.waitForExplicitFinish();
> +
> +SpecialPowers.pushPrefEnv({'set': [['layout.css.prefixes.webkit', false],

This pref would be removed at some point I believe, so in this way, we will need to update this test every time we remove a pref being tested here.

I'd suggest we test that, every properties in the list below is not disabled, so that we don't need to mess with some specific pref here. I think you can get the list from property_database.js.

You should still manually check that this test does fail (with this specific pref disabled) before your code change.

BTW, I'm not a style peer, so you may want to ask heycam to review this as well.
https://reviewboard.mozilla.org/r/47935/#review44675

> Probably you should say explicitly that, transition would not be applied to any property which is not available to the content.

I mean, "flags are ignored" is rather vague that one may not be able to understand the impact immediately.
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #10)
> ::: layout/style/nsCSSProps.h:229
> (Diff revision 1)
> > +// Furthermore, for the purposes of animation (including triggering
> > +// transitions) these flags are ignored.
> 
> Is animation affected here? The code change below doesn't imply this to me.
> It's probably worth trying using "all" property in @keyframes and see what
> would happen.

No, this is just documenting the existing behavior. "all" doesn't apply to @keyframes.

> ::: layout/style/test/test_transitions_with_disabled_properties.html:28
> (Diff revision 1)
> > +<div id="display"></div>
> > +<pre id="test">
> > +<script>
> > +SimpleTest.waitForExplicitFinish();
> > +
> > +SpecialPowers.pushPrefEnv({'set': [['layout.css.prefixes.webkit', false],
> 
> This pref would be removed at some point I believe, so in this way, we will
> need to update this test every time we remove a pref being tested here.

Will we? I thought we'd always want to be able to disable webkit prefixes?

> I'd suggest we test that, every properties in the list below is not
> disabled, so that we don't need to mess with some specific pref here. I
> think you can get the list from property_database.js.

I don't think property_database.js can tell us about disabled properties, only enabled ones. Also, I'd specifically like to test properties controlled by a pref and layout.css.prefixes.webkit seems like that most likely to prefix to remain in the tree (maybe layout.css.osx-font-smoothing.enabled?).
 
> You should still manually check that this test does fail (with this specific
> pref disabled) before your code change.

As per the changeset comment, this test does not fail, it only generates a warning. It is added as a regression test.
Attachment #8743617 - Flags: review?(cam)
(In reply to Brian Birtles (:birtles) from comment #12)
> (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #10)
> 
> > ::: layout/style/test/test_transitions_with_disabled_properties.html:28
> > (Diff revision 1)
> > > +<div id="display"></div>
> > > +<pre id="test">
> > > +<script>
> > > +SimpleTest.waitForExplicitFinish();
> > > +
> > > +SpecialPowers.pushPrefEnv({'set': [['layout.css.prefixes.webkit', false],
> > 
> > This pref would be removed at some point I believe, so in this way, we will
> > need to update this test every time we remove a pref being tested here.
> 
> Will we? I thought we'd always want to be able to disable webkit prefixes?

I think we will. WebKit prefixes are being part of the platform, and we are not able to remove them in the future. We do not remove quirks mode either, right?

> > I'd suggest we test that, every properties in the list below is not
> > disabled, so that we don't need to mess with some specific pref here. I
> > think you can get the list from property_database.js.
> 
> I don't think property_database.js can tell us about disabled properties,
> only enabled ones. Also, I'd specifically like to test properties controlled
> by a pref and layout.css.prefixes.webkit seems like that most likely to
> prefix to remain in the tree (maybe layout.css.osx-font-smoothing.enabled?).

Yes, and I suggested we should test that *every* properties in the transition properties list is enabled.

We can probably have a list of prefs which are controlling properties and switching off all of them before running the test, but we cannot force people adding new prefs to that list I guess...

> > You should still manually check that this test does fail (with this specific
> > pref disabled) before your code change.
> 
> As per the changeset comment, this test does not fail, it only generates a
> warning. It is added as a regression test.

If the test does never fail before the change, I don't think adding this test makes any sense...
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #13)
> (In reply to Brian Birtles (:birtles) from comment #12)
> > (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #10)
> > > This pref would be removed at some point I believe, so in this way, we will
> > > need to update this test every time we remove a pref being tested here.
> > 
> > Will we? I thought we'd always want to be able to disable webkit prefixes?
> 
> I think we will. WebKit prefixes are being part of the platform, and we are
> not able to remove them in the future. We do not remove quirks mode either,
> right?

Right, but I imagined it would still be useful to be able to temporarily switch this off? It seems useful to people doing web compat work or authors testing their content.

> > > I'd suggest we test that, every properties in the list below is not
> > > disabled, so that we don't need to mess with some specific pref here. I
> > > think you can get the list from property_database.js.
> > 
> > I don't think property_database.js can tell us about disabled properties,
> > only enabled ones. Also, I'd specifically like to test properties controlled
> > by a pref and layout.css.prefixes.webkit seems like that most likely to
> > prefix to remain in the tree (maybe layout.css.osx-font-smoothing.enabled?).
> 
> Yes, and I suggested we should test that *every* properties in the
> transition properties list is enabled.

How does that help us test disabled properties? property_database.js won't report disabled properties. They're not added to the database if the pref isn't set.

To make this truly automated the best I can think of is:

1. Maintain a list of all prefs that conditionally turn off CSS properties.
2. Turn them all on.
3. Enumerate all the properties using property_database.js
4. Turn them all off.
5. Enumerate all the properties using property_database.js
6. Compare the lists from (3) and (5). Anything in (3) but not in (5) is a disabled property.
7. If there are no properties that in (3) but not in (5) abort with a warning that we have no disabled properties.
8. Choose one of the disabled properties and run a test with it.

That's quite involved but maybe that's what we have to do. I'll look into it tomorrow but I might do it in a follow-up bug since we really need to fix this bug before the uplift since it is the most likely cause of crash bug 1264863.

> > > You should still manually check that this test does fail (with this specific
> > > pref disabled) before your code change.
> > 
> > As per the changeset comment, this test does not fail, it only generates a
> > warning. It is added as a regression test.
> 
> If the test does never fail before the change, I don't think adding this
> test makes any sense...

It's added as a regression test. We may, for example, tweak nsTransitionManager.cpp such that we begin triggering transitions for disabled properties. In that case this test will catch it. That's the motivation behind parts 4 and 5 in this patch series too. Furthermore it ensures the added code path is traversed and doesn't have any dramatic side effects. Further comments are in the changeset header.
(In reply to Brian Birtles (:birtles) from comment #14)
> (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #13)
> > Yes, and I suggested we should test that *every* properties in the
> > transition properties list is enabled.
> 
> How does that help us test disabled properties? property_database.js won't
> report disabled properties. They're not added to the database if the pref
> isn't set.

If every property in transitionedProperties is enabled, then none of them is disabled, right?
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #15)
> (In reply to Brian Birtles (:birtles) from comment #14)
> > (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #13)
> > > Yes, and I suggested we should test that *every* properties in the
> > > transition properties list is enabled.
> > 
> > How does that help us test disabled properties? property_database.js won't
> > report disabled properties. They're not added to the database if the pref
> > isn't set.
> 
> If every property in transitionedProperties is enabled, then none of them is
> disabled, right?

For part 3 we only need to test that we don't trigger transitions on dependent properties that are disabled, but for parts 4 and 5 we actually want to try and test a disabled property. So we need a way to actually find a disabled property.
Comment on attachment 8743615 [details]
MozReview Request: Bug 1265611 - Make TransitionProperty() and ToValue() safe when mProperties is not set

https://reviewboard.mozilla.org/r/47931/#review44729
Attachment #8743615 - Flags: review?(cam) → review+
Comment on attachment 8743616 [details]
MozReview Request: Bug 1265611 - Crashtest

https://reviewboard.mozilla.org/r/47933/#review44731

::: layout/style/crashtests/crashtests.list:142
(Diff revision 1)
>  load 1233135-2.html
>  load 1238660-1.html
>  load 1245260-1.html
>  load 1247865-1.html
>  load 1264396-1.html
> +pref(layout.css.prefixes.webkit,false) load 1265611-1.html

Maybe add a comment here saying that this test relies on -webkit-text-fill-color being behind the pref?  And then a more substantial comment in the test?  If the pref ever disappears (probably not, as you say, but who knows) the test will need to be rewritten.
Attachment #8743616 - Flags: review?(cam) → review+
Comment on attachment 8743617 [details]
MozReview Request: Bug 1265611 - Don't trigger transitions for properties that are disabled

https://reviewboard.mozilla.org/r/47935/#review44741

::: layout/style/test/test_transitions_with_disabled_properties.html:28
(Diff revision 1)
> +<div id="display"></div>
> +<pre id="test">
> +<script>
> +SimpleTest.waitForExplicitFinish();
> +
> +SpecialPowers.pushPrefEnv({'set': [['layout.css.prefixes.webkit', false],

I think given the difficulty involved in knowing which prefs control enabling properties, and what the current list of disable properties is, what you have here is OK.  But I would add a comment in here describing why we're using the webkit pref, so that if we eventually do remove the pref, the test can be updated.

(If in a followup you do find a more systematic way to test this, then that comment could be removed then.)
Attachment #8743617 - Flags: review?(cam) → review+
Comment on attachment 8743618 [details]
MozReview Request: Bug 1265611 - Add tests for CSS animations

https://reviewboard.mozilla.org/r/47937/#review44743

FWIW (and you probably realise this and made a conscious choice here) you could avoid splitting the test over two files just inserting the style sheet after pushing the pref.  (Either by filling in the style sheet text content, or maybe having the style sheet text content in there already and doing a |document.querySelector("style").textContent += " ";| or something.)

::: layout/style/test/test_animations_with_disabled_properties.html:23
(Diff revision 1)
> +SpecialPowers.pushPrefEnv(
> +  { 'set': [[ 'dom.animations-api.core.enabled', true ],
> +            [ 'layout.css.prefixes.webkit', false ]] },

Please add a comment in here too about the choice of pref/property.
Attachment #8743618 - Flags: review?(cam) → review+
Attachment #8743619 - Flags: review?(cam) → review+
Comment on attachment 8743619 [details]
MozReview Request: Bug 1265611 - Add tests that we ignore disabled properties when creating animations

https://reviewboard.mozilla.org/r/47939/#review44747

::: dom/animation/test/mozilla/file_disabled_properties.html:13
(Diff revision 1)
> +function waitForSetPref(pref, value) {
> +  return new Promise(function(resolve, reject) {
> +    SpecialPowers.pushPrefEnv({ 'set': [[pref, value]] }, resolve);
> +  });
> +}
> +

And again a comment in here too.
https://reviewboard.mozilla.org/r/47939/#review44747

::: dom/animation/test/mozilla/file_disabled_properties.html:13
(Diff revision 1)
> +function waitForSetPref(pref, value) {
> +  return new Promise(function(resolve, reject) {
> +    SpecialPowers.pushPrefEnv({ 'set': [[pref, value]] }, resolve);
> +  });
> +}
> +

And again a comment in here too.
(In reply to Cameron McCormack (:heycam) from comment #20)
> Comment on attachment 8743618 [details]
> MozReview Request: Bug 1265611 - Add tests for CSS animations
> 
> https://reviewboard.mozilla.org/r/47937/#review44743
> 
> FWIW (and you probably realise this and made a conscious choice here) you
> could avoid splitting the test over two files just inserting the style sheet
> after pushing the pref.  (Either by filling in the style sheet text content,
> or maybe having the style sheet text content in there already and doing a
> |document.querySelector("style").textContent += " ";| or something.)

Actually it was more about muscle memory since for the Web Animations tests where the pref controls what gets added to the global we need to split it over two files. Also, I didn't know about the .textContent += " " trick to trigger reparsing of the stylesheet.

Since we don't actually need any of the Web Animations globals in this test, I've updated it to put everything in the one file. Thanks!
Depends on: 1267018
You need to log in before you can comment on or make changes to this bug.