Closed Bug 1402157 Opened 7 years ago Closed 7 years ago

stylo: Assertion failure: aFrame->HasImageRequest() (why call me?) in [@ mozilla::css::ImageLoader::DisassociateRequestFromFrame]

Categories

(Core :: CSS Parsing and Computation, defect, P2)

Product:
Core
Component:
CSS Parsing and Computation
Version:
56 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- disabled
firefox57 + wontfix
firefox58 --- fixed

People

(Reporter: tsmith, Assigned: u459114)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, testcase)

Attachments

(5 files, 2 obsolete files)

test_case.html
724 bytes, text/html
Details
simplified testcase
362 bytes, text/html
Details
Bug 1402157 - Disassociate a frame with an image loader if that frame indeed has any image request.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1402157 - Part 2. A crash test for disassociating image loaders and frames.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1402157 - Test
59 bytes, text/x-review-board-request
Details
Attached file test_case.html — 
Assertion failure: aFrame->HasImageRequest() (why call me?), at /builds/worker/workspace/build/src/layout/style/ImageLoader.cpp:183

#0 0x7f4141115bd2 in mozilla::css::ImageLoader::DisassociateRequestFromFrame(imgIRequest*, nsIFrame*) /src/layout/style/ImageLoader.cpp:183:3
#1 0x7f414162959f in std::_Function_handler<void (imgRequestProxy*), AddAndRemoveImageAssociations(nsFrame*, nsStyleImageLayers const*, nsStyleImageLayers const*)::$_7>::_M_invoke(std::_Any_data const&, imgRequestProxy*) /src/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.9.4/../../../../include/c++/4.9.4/functional:2039:2
#2 0x7f414162941c in std::function<void (imgRequestProxy*)>::operator()(imgRequestProxy*) const /src/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.9.4/../../../../include/c++/4.9.4/functional:2440:14
#3 0x7f4141628f78 in CompareLayers(nsStyleImageLayers const*, nsStyleImageLayers const*, std::function<void (imgRequestProxy*)> const&) /src/layout/generic/nsFrame.cpp:854:9
#4 0x7f41415e53fb in AddAndRemoveImageAssociations(nsFrame*, nsStyleImageLayers const*, nsStyleImageLayers const*) /src/layout/generic/nsFrame.cpp:877:5
#5 0x7f414159fb5b in nsFrame::DidSetStyleContext(nsStyleContext*) /src/layout/generic/nsFrame.cpp:922:3
#6 0x7f41413a2450 in nsIFrame::SetStyleContext(nsStyleContext*) /src/layout/generic/nsIFrame.h:770:7
#7 0x7f41416c1842 in nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit(mozilla::ServoRestyleState&) /src/layout/generic/nsInlineFrame.cpp:1061:13
#8 0x7f4141610abb in nsIFrame::DoUpdateStyleOfOwnedAnonBoxes(mozilla::ServoRestyleState&) /src/layout/generic/nsFrame.cpp:10651:42
#9 0x7f41414090fb in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:877:19
#10 0x7f414140934d in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:920:32
#11 0x7f414140bb2c in mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:1121:28
#12 0x7f41413d736c in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /src/layout/base/PresShell.cpp:4170:41
#13 0x7f414136bf73 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:1921:18
#14 0x7f414137548e in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /src/layout/base/nsRefreshDriver.cpp:307:7
#15 0x7f4141375264 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:328:5
#16 0x7f4141378855 in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:770:5
#17 0x7f41413778f6 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:683:35
#18 0x7f4141373867 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() /src/layout/base/nsRefreshDriver.cpp:529:20
#19 0x7f413b9e471f in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1039:14
#20 0x7f413b9e9a90 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:521:10
#21 0x7f413c55a865 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
#22 0x7f413c4af7a7 in MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10
#23 0x7f413c4af639 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3
#24 0x7f4140e6b85a in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:158:27
#25 0x7f4144086641 in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30
#26 0x7f41441eab01 in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4701:22
#27 0x7f41441ec7ca in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4865:8
#28 0x7f41441ed6b8 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4960:21
#29 0x4ed398 in do_main(int, char**, char**) /src/browser/app/nsBrowserApp.cpp:236:22
#30 0x4eccb0 in main /src/browser/app/nsBrowserApp.cpp:309:16
#31 0x7f415ab4282f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
#32 0x41e9e4 in _start (firefox+0x41e9e4)
Flags: in-testsuite?
I can't reproduce it in any of my local Linux64 debug builds.
Reproduces on Win10 for me, but not on Ubuntu 17.04. And only with Stylo enabled.

INFO: Last good revision: 8b274bfe790511b3517f9dc98f54f56efc8dee7c
INFO: First bad revision: 319bc66e5d3a6599a61b686bc58ee399dac77b65
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8b274bfe790511b3517f9dc98f54f56efc8dee7c&tochange=319bc66e5d3a6599a61b686bc58ee399dac77b65
Blocks: 1301245
Has Regression Range: --- → yes
status-firefox56: --- → disabled
status-firefox-esr52: --- → unaffected
Flags: needinfo?(cku)
Priority: -- → P2
Summary: Assertion failure: aFrame->HasImageRequest() (why call me?) in [@ mozilla::css::ImageLoader::DisassociateRequestFromFrame] → stylo: Assertion failure: aFrame->HasImageRequest() (why call me?) in [@ mozilla::css::ImageLoader::DisassociateRequestFromFrame]
Version: Trunk → 56 Branch
I can reproduce it with my local windows debug build, and there seems to be another bug that both processes can be hanged with a reduced testcase.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> I can reproduce it with my local windows debug build, and there seems to be
> another bug that both processes can be hanged with a reduced testcase.

That one is because we repeatedly call load() in onerror for an invalid url, which is basically a infinite loop, so probably not a bug.

But this does help me understand this testcase.
Attached file simplified testcase (obsolete) — 
A somehow simplified testcase.
Attached file simplified testcase — 

        Attachment #8916461 -
        Attachment is obsolete: true

    
    

    
  
Given this testcase, I suppose that it is related to resolution of fragment url with location change.

Not sure what's happening here. CJKu may know more details.

    
    

    
  
Assigned to CJ to figure out.
Assignee: nobody → cku
Status: NEW → ASSIGNED

    
    

    
  
[Tracking Requested - why for this release]:

We might want to track this for 57 because this is a reproducible process hang.

          tracking-firefox57:
          --- → ?

    
    

    
  
(In reply to Chris Peterson [:cpeterson] from comment #9)
> [Tracking Requested - why for this release]:
> 
> We might want to track this for 57 because this is a reproducible process
> hang.

Tracking 57+ for this issue.

          tracking-firefox57:
          ? → +

    
    

    
  

      
      
      
      

        Attached file
          
        Test.html (obsolete)
        — 
      

    


  
Flags: needinfo?(cku)

    
  

        Attachment #8918181 -
        Attachment is obsolete: true

    
    

    
      
  


  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
  

        Attachment #8918189 -
        Flags: review?(cam)

        Attachment #8918212 -
        Flags: review?(cam)

    
    

    
  
Comment on attachment 8918189 [details]
Bug 1402157 - Disassociate a frame with an image loader if that frame indeed has any image request.

https://reviewboard.mozilla.org/r/189068/#review195236

Sorry for the delay.

::: commit-message-2ca37:8
(Diff revision 3)
> +Before enable stylo, an image request is always been resolved before the
> +creation time of that request object[1].
> +As a result, in AddAndRemoveImageAssociations, we always associate the request
> +objects of style images with the given frame(since they are resolved).
> +
> +After enable stylo, an image request is resolved after tha object been created[2].

s/tha/that/

        Attachment #8918189 -
        Flags: review?(cam) → review+

    
    

    
  
Comment on attachment 8918212 [details]
Bug 1402157 - Part 2. A crash test for disassociating image loaders and frames.

https://reviewboard.mozilla.org/r/189090/#review195240

::: commit-message-75994:1
(Diff revision 2)
> +Bug 1402157 - Part 2. A crash test for disassociate image loaders and frames.

s/disassociate/disassociating/

        Attachment #8918212 -
        Flags: review?(cam) → review+

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  


  

    
    

    
  
(In reply to Marcia Knous [:marcia - use ni. PTO 10/18-10/20] from comment #10)
> (In reply to Chris Peterson [:cpeterson] from comment #9)
> > [Tracking Requested - why for this release]:
> > 
> > We might want to track this for 57 because this is a reproducible process
> > hang.
> 
> Tracking 57+ for this issue.
It's not a process hang, it's an assertion failure. Browser can still work correctly in release version. I don't think we need to uplift these patches to 57.

    
    

    
      
  

    
    

    
      
  

    
    

    
  
https://hg.mozilla.org/integration/mozilla-inbound/rev/14969c3ddcffbfc636be9c49c39a620160846f5f
Bug 1402157 - Disassociate a frame with an image loader if that frame indeed has any image request. r=heycam

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/14969c3ddcff
Status: ASSIGNED → RESOLVED
Closed: 7 years ago

          status-firefox58:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

          status-firefox57:
          affected → wontfix
Flags: in-testsuite? → in-testsuite+

    
    

    
  
Did we not land the tests on purpose?
Flags: needinfo?(cku)

    
    

    
  
(In reply to Ryan VanderMeulen [:RyanVM] from comment #27)
> Did we not land the tests on purpose?
Yes, that test case can not be loaded correctly in reftest harness.
Flags: needinfo?(cku)

    
    

    
  
What about the crashtest?

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: