Loading forever when open a page which contains multipart image/motion jpeg streams (Webcam)

RESOLVED FIXED

Status

()

RESOLVED FIXED
8 years ago
3 years ago

People

(Reporter: alice0775, Assigned: bholley)

Tracking

({regression})

Trunk
x86
Windows 7
regression
Points:
---

Firefox Tracking Flags

(blocking2.0 final+)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a6pre) Gecko/20100628 Minefield/3.7a6pre ID:20100628035917
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a6pre) Gecko/20100628 Minefield/3.7a6pre ID:20100628035917

Loading forever when open a page which contains multipart image/motion jpeg streams(Webcam).
Progress meter in the statusbar and in the tab indicate not 100% and not disappear.

Tjis problem does not happens on Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.7pre) Gecko/20100626 Namoroka/3.6.7pre ID:20100626041846.

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Open URL ( http://www.upl.cs.wisc.edu/w/Webcam )

Actual Results:
 Progress meter in the statusbar and in the tab indicates not 100%.
 And loading forever.

Expected Results:
 Progress meter in the statusbar and in the tab should indicate 100% and should disappear after the leading completion of the page.

Works(Progress meter disappears after complete page loading, but the image flickers)
http://hg.mozilla.org/mozilla-central/rev/ff3496b1f6c7
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20090912 Minefield/3.7a1pre ID:20090912042051

Fails(Loading forever):
http://hg.mozilla.org/mozilla-central/rev/bf0fdec8f43b
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20090913 Minefield/3.7a1pre ID:20090913050738

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ff3496b1f6c7&tochange=bf0fdec8f43b
(Reporter)

Comment 1

8 years ago
I guess it is regression of Bug 512435 - document onload should not fire until all visible images have 1 decoded frame
Component: General → ImageLib
Keywords: regression
Product: Firefox → Core
QA Contact: general → imagelib
Summary: Loading forever when open a page which contains multipart image/motion jpeg streams(Webcam) → Loading forever when open a page which contains multipart image/motion jpeg streams (Webcam)

Comment 2

8 years ago
No issues for me. The load bar shouldn't stop, because it is a live picture, which basically means it is downloading all the time.
(Reporter)

Comment 3

8 years ago
(In reply to comment #2)
> No issues for me. The load bar shouldn't stop, because it is a live picture,
> which basically means it is downloading all the time.

Namoroka 3.6.7pre and Google Chrome: the progress bar disappears.
And HTML5 video, Flash movie and etc... : the  progress bar also disappears though it is downloading stream data.
So, I think the Webcam sterem should be  also same handling  as other streaming.
Yeah, this isn't something we should be shipping...
Blocks: 512435
blocking2.0: --- → ?
Just looked into this. What appears to be happening is that the multipart image fires the cycle of notifications continuously, all of which are received by nsImageLoadingContent. When OnStartDecode() is received, SetBlockingOnload(PR_TRUE) gets called, and the load is blocked. When OnStopFrame() is received, SetBlockingOnload(PR_FALSE) gets called, and the load is unblocked. However, this doesn't seem to have enough time to take effect before the next OnStartDecode() is received, so we never completely finish loading the document.

Disabling the machinery in SetBlockingOnload() fixes this problem.

Bz - does this all make sense? What would you propose as a solution?
Status: NEW → ASSIGNED
> SetBlockingOnload(PR_TRUE) gets called, and the load is blocked. When
> OnStopFrame() is received, SetBlockingOnload(PR_FALSE) gets called, and the
> load is unblocked. 

This all makes sense.

> However, this doesn't seem to have enough time to take effect before the next
> OnStartDecode() is received

Yeah, since the unblock here is async.

> What would you propose as a solution?

Not blocking onload for the latter parts of a multipart image.  We try hard in imagelib to do that (see the LOAD_BACKGROUND stuff in imgRequestProxy).  This means the image loading content needs to be able to tell whether it's looking at such a latter part of a multipart image.
Created attachment 455287 [details] [diff] [review]
patch v1

Added a patch that seems to do the trick.

Flagged bz for review, and pushed to try as 4840b879ad03.
Assignee: nobody → bobbyholley+bmo
Attachment #455287 - Flags: review?(bzbarsky)
Comment on attachment 455287 [details] [diff] [review]
patch v1

Good catch on the load flag.  LOAD_BACKGROUND requests should never block onload, so we just don't want to block for them, period, no matter how they got to be LOAD_BACKGROUND.  Just say that and nix the part about multipart.

r- because you need to fix OnStopDecode to avoid unbalanced unblocks, no?
Attachment #455287 - Flags: review?(bzbarsky) → review-
Created attachment 455301 [details] [diff] [review]
patch v2

(In reply to comment #8)
> Just say that and nix the part about multipart.

Done.

> r- because you need to fix OnStopDecode to avoid unbalanced unblocks, no?

I don't think so. SetBlockingOnload(PR_FALSE) only does something if SetBlockingOnload(PR_TRUE) was called previously (this is the reason for the slightly awkward naming). So the rest of those calls should just be no-ops.

This could use some test coverage. We had a bug somewhere about multipart/x-mixed-replace test coverage, but I can't find it. I've emailed joe about it.
Attachment #455287 - Attachment is obsolete: true
Attachment #455301 - Flags: review?(bzbarsky)
Comment on attachment 455301 [details] [diff] [review]
patch v2

>+  if (NS_SUCCEEDED(rv) && (loadFlags & nsIRequest::LOAD_BACKGROUND))
>+    background = PR_TRUE;

How about:

  PRBool background = 
    (NS_SUCCEEDED(rv) && (loadFlags & nsIRequest::LOAD_BACKGROUND));

>+  // Block onload if it's the current request and not background
>+  if ((aRequest == mCurrentRequest) && !background) {

Please don't overparenthesize the == check.  And maybe do the whole flag-getting stuff only if aRequest == mCurrentRequest, unless there's a good reason to do it in all cases?

r=bzbarsky with those nits.
Attachment #455301 - Flags: review?(bzbarsky) → review+
Created attachment 455331 [details] [diff] [review]
patch v3

Attached the patch I'm going to push. I'll let it run on try first.
pushed to try as 33429b3d126a.

Comment 13

8 years ago
Please STOP posting here. I have been getting 6 emails TODAY, only to find out all of them where actually from this thread and Mozilla Bugzilla. This is not damn funny at all. PLEASE STOP NOW!

Comment 14

8 years ago
No. I NEVER added myself to the cc list of this bug, I believe. It probably happened automatically after I made my first post in this thread. Thanks.
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/255acc6edc7b

Resolving fixed.

*Ducks*
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
blocking2.0: ? → final+
You need to log in before you can comment on or make changes to this bug.