Closed Bug 1120036 Opened 5 years ago Closed 5 years ago

tab-throbber stops spinning

Categories

(Core :: ImageLib, defect)

37 Branch
x86_64
All
defect
Not set

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox36 --- unaffected
firefox37 --- affected
firefox38 --- fixed

People

(Reporter: alice0775, Unassigned)

References

Details

(Keywords: regression, Whiteboard: gfx-noted [fixed by 1079627])

[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
Component: General → ImageLib
Product: Firefox → Core
Alternate str
1. Open chrome://browser/skin/tabbrowser/loading.png
   or
   Open chrome://browser/skin/tabbrowser/connecting.png

Actual Results:
 tab-throbber stops spinning.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1119938
Fixed by bug 1117607
Resolution: DUPLICATE → WORKSFORME
Whiteboard: [ Fixed by bug 1117607 ]
(In reply to Alice0775 White from comment #3)
> Fixed by bug 1117607

That was backed out, it seems...
(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)
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Whiteboard: [ Fixed by bug 1117607 ]
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
See Also: → 1119938
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
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → WORKSFORME
Whiteboard: [Fixed by Bug 1079627]
 Backed out bug 1079627
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Whiteboard: [Fixed by Bug 1079627]
(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. :-)
Flags: needinfo?(alice0775)
(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)
Flags: needinfo?(alice0775)
Sounds like it depends on bug 1079627, or is at least fixed by it?
Whiteboard: gfx-noted
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.)
Try STR in comment #1 to reproduce this easily
(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. =(
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
OS: Windows 7 → All
OK, opening the same page in several tabs is something I haven't tried! I'll give that a shot.
Duplicate of this bug: 1122318
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.
For those of us that can reproduce what kind of info would help to narrow down the problem?
(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.
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.
This seems to be already fixed by Bug 1079627.

Fixed range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=53d0b45c24c5&tochange=5feaa9fcb009
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → WORKSFORME
Whiteboard: gfx-noted → gfx-noted [fixed by 1079627]
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.
You need to log in before you can comment on or make changes to this bug.