1120036 - tab-throbber stops spinning

Status: RESOLVED WORKSFORME

(Core :: Graphics: ImageLib, defect)

Description

[Alice0775 White]

[Tracking Requested - why for this release]:

Steps to reproduce:
1. open any page (e.g., https://developer.mozilla.org/en-US/Add-ons/Code_snippets )
2. Reload(f5) if necessary

Actual Results:
 stops spinning

Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0933c1aef197
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0 ID:20150108000307
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4cdc04f6555
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0 ID:20150108000517
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0933c1aef197&tochange=b4cdc04f6555
Triggered by: Bug 1116733

Comment 1

[Alice0775 White]

Alternate str
1. Open chrome://browser/skin/tabbrowser/loading.png
   or
   Open chrome://browser/skin/tabbrowser/connecting.png

Actual Results:
 tab-throbber stops spinning.

Comment 3

[Alice0775 White]

Fixed by bug 1117607

Comment 4

[:Gijs (he/him)]

(In reply to Alice0775 White from comment #3)
> Fixed by bug 1117607

That was backed out, it seems...

Comment 5

[Alice0775 White]

(In reply to :Gijs Kruitbosch from comment #4)
> (In reply to Alice0775 White from comment #3)
> > Fixed by bug 1117607
> 
> That was backed out, it seems...

yes, it was backed out....


And bug 1119938 is different seems regression range (bug 1119938 Comment#3)

Comment 6

[Alice0775 White]

Still reproduce the problem


https://hg.mozilla.org/integration/mozilla-inbound/rev/8f3cc2c90893
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0 ID:20150111114930

Comment 7

[Alice0775 White]

Fixed by Bug 1079627

https://hg.mozilla.org/integration/mozilla-inbound/rev/14cc155b0d6e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0 ID:20150112012130

Comment 8

[Alice0775 White]

 Backed out bug 1079627

Comment 9

[:Gijs (he/him)]

(In reply to Alice0775 White from comment #8)
>  Backed out bug 1079627

Would it be OK to just dupe this back to bug 1119938? See bug 1119938 comment 9. Then we can track this issue in one place. :-)

Comment 10

[Alice0775 White]

(In reply to :Gijs Kruitbosch from comment #9)
> (In reply to Alice0775 White from comment #8)
> >  Backed out bug 1079627
> 
> Would it be OK to just dupe this back to bug 1119938? See bug 1119938
> comment 9. Then we can track this issue in one place. :-)

I do not think dup.

bug 1119938 is different regression range (bug 1119938 Comment#3)

Comment 11

[Milan Sreckovic [:milan] (needinfo for best results)]

Sounds like it depends on bug 1079627, or is at least fixed by it?

Comment 12

[Seth Fowler [:seth] [:s2h]]

There are two things I currently think may have caused this bug:

1. Lack of thread safety in VolatileBuffer. Unfortunately I overlooked this issue when I landed the original patch to start allocating imgFrame objects off the main thread. (Bug 1116733.) I've posted a patch that should fix this in bug 1121297, which got reviewed last night. I'll push that shortly.

2. We switched to notifying that new frames had been added to the image asynchronously instead of synchronously. (This was also done in bug 1116733, via the new notification method RasterImage::OnAddedFrame.) It's not as obvious to me why this would be a problem for animated images, but it seems to have been the cause of the backout of bug 1117607, because once we started doing it for the first frame it was possible for RasterImage::DecodingComplete to get called *before* RasterImage::OnAddedFrame, which confused some image notifications observers.

If the patch for case 1 doesn't fix this issue, I'll focus on debugging case 2. Unfortunately, it's pretty hard for me to tell whether a given patch fixes the issue, because I have a lot of trouble reproducing this locally. (I have been able to sometimes, though.)

Comment 13

[Alice0775 White]

Try STR in comment #1 to reproduce this easily

Comment 14

[Seth Fowler [:seth] [:s2h]]

(In reply to Alice0775 White from comment #13)
> Try STR in comment #1 to reproduce this easily

I wish that were true. It works almost 100% of the time on my machine. =(

Comment 15

[Alice0775 White]

And I can reproduce on Ubuntu14.04 too.
https://hg.mozilla.org/mozilla-central/rev/c1f6345f2803
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0 ID:20150115030228

Steps to reproduce:
1. Open MDN https://developer.mozilla.org/en-US/ etc in several tabs

2. Reload all tabs if need
   

Actual Results:
Loading(clockwise) throbber stops

Comment 16

[Seth Fowler [:seth] [:s2h]]

OK, opening the same page in several tabs is something I haven't tried! I'll give that a shot.

Comment 18

[Seth Fowler [:seth] [:s2h]]

See bug 1119938 comment 16 for my current take on this issue.

FWIW, I didn't have any luck reproducing this so far, even with the same page in several tabs.

Comment 19

[Ben Turner (not reading bugmail, use the needinfo flag!)]

For those of us that can reproduce what kind of info would help to narrow down the problem?

Comment 20

[Seth Fowler [:seth] [:s2h]]

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #19)
> For those of us that can reproduce what kind of info would help to narrow
> down the problem?

It would be very helpful to determine the state of the RasterImage whose animation freezes and its associated FrameAnimator.

Adding this code to RasterImage::Init will print out the address of each RasterImage and its URL:

>  ImageURL* url(GetURI());
>  if (url) {
>    nsAutoCString spec;
>    url->GetSpec(spec);
>    printf("*** %p RasterImage::Init: [%s]\n", this, spec.get());
>  }

Then, once you manage to get an animation to freeze (either the tab throbber or some other animated GIF or PNG; there's nothing special about the tab throbber), find the address of the corresponding RasterImage instance in the log, attach gdb, and print out the member variables of the RasterImage itself and its FrameAnimator (RasterImage::mAnim).

I'd also be quite interested to know if you can reproduce this on a fresh profile. Daniel told me that he couldn't, which might just be because there's less going on in a fresh browser session, but could also indicate that addons are part of the issue here.

Comment 21

[Seth Fowler [:seth] [:s2h]]

There's also a patch in bug 1119938 comment 16 which you can try out; it'd be helpful if someone can verify that it works or doesn't work.

Comment 22

[Alice0775 White]

This seems to be already fixed by Bug 1079627.

Fixed range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=53d0b45c24c5&tochange=5feaa9fcb009

Comment 23

[Seth Fowler [:seth] [:s2h]]

Thanks, Alice0775!

Ben, if you can still reproduce, let's take the discussion to bug 1119938. It makes perfect sense to me that this issue would be fixed by bug 1079627, so I think any lingering issues are probably a different bug, just as Alice0775 stated in comment 10.