Enable getting updates on how many bytes of an appcache update have been downloaded

RESOLVED FIXED in mozilla15

Status

()

Core
Networking: Cache
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sicking, Assigned: mayhemer)

Tracking

Trunk
mozilla15
Points:
---

Firefox Tracking Flags

(blocking-kilimanjaro:+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Currently you can only get information about how many items of an appcache has been downloaded. You can't get information about how many bytes have been downloaded. This means that if you want to display a progress bar, you have to guess that all resources are of the same size, which can result in a very unreliable progress bar.

Would be great to expose this information through for example a new function on nsIOfflineCacheUpdateObserver
Blocks: 744712
Over to Honza in hopes that he can help out here.
Assignee: nobody → honzab.moz
(Assignee)

Comment 2

5 years ago
From point of view of web content, we have to have a new event (mozByteProgress maybe) that will fire with some reasonable granularity and give the overall number of bytes so far downloaded.  Depends on if this is needed, but maybe now according the description.

From point of view of C++ nsIOfflineCacheUpdateObserver can have a new event implemented for which updateStateChanges can be fired (like STATE_ITEMPROGRESS).  nsIOfflineCacheUpdate then can expose the overall number of bytes downloaded through a new r/o attribute.

Items already expose the number of downloaded bytes via nsIDOMLoadStatus (loadedSize).  But items are not exposed anywhere.
This blocks our webapps story, so blocking kilimanjaro.
blocking-kilimanjaro: --- → +
I wouldn't worry about exposing this to web content yet. It's enough to expose it to our internal APIs and then we'll expose it through to webapp installation api.

I certainly think that we should expose it to web content eventually too, but that's lower priority and I think we need to come up with a comprehensive plan for how we want to fix the appcache for web content anyway.
Agreed, especially given the tight schedule. The less we do here the more time we have for the other work, so let's delay exposing this to web content for now.
(Assignee)

Comment 6

5 years ago
Created attachment 621038 [details] [diff] [review]
v1
Attachment #621038 - Flags: review?(dcamp)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
Attachment #621038 - Flags: review?(dcamp) → review?(michal.novotny)
Comment on attachment 621038 [details] [diff] [review]
v1

>    const unsigned long STATE_ITEMCOMPLETED = 7;
> +  const unsigned long STATE_ITEMPROGRESS = 11;
>    const unsigned long STATE_FINISHED = 10;

Why did you choose 11?


> +    if (mUpdate)
> +        mUpdate->OnByteProgress(bytesRead);

AFAICS this can never be null. And if it could then we would miss some progress update so nsOfflineCacheUpdate::mByteProgress would not be correct.


> +OfflineCacheUpdateChild::GetByteProgress(PRUint64 * _result)
> +{
> +    NS_ENSURE_ARG(_result);
> +    // TODO Make this available on e10s content processes too
> +    *_result = 0;
> +}

Shouldn't you return mByteProgress here?
(Assignee)

Comment 8

5 years ago
(In reply to Michal Novotny (:michal) from comment #7)
> Comment on attachment 621038 [details] [diff] [review]
> v1
> 
> >    const unsigned long STATE_ITEMCOMPLETED = 7;
> > +  const unsigned long STATE_ITEMPROGRESS = 11;
> >    const unsigned long STATE_FINISHED = 10;
> 
> Why did you choose 11?

Just to be save we don't overlap with anything older.

> 
> 
> > +    if (mUpdate)
> > +        mUpdate->OnByteProgress(bytesRead);
> 
> AFAICS this can never be null. And if it could then we would miss some
> progress update so nsOfflineCacheUpdate::mByteProgress would not be correct.

Yes, there is no need for a non-null check.

> 
> 
> > +OfflineCacheUpdateChild::GetByteProgress(PRUint64 * _result)
> > +{
> > +    NS_ENSURE_ARG(_result);
> > +    // TODO Make this available on e10s content processes too
> > +    *_result = 0;
> > +}
> 
> Shouldn't you return mByteProgress here?

Ha.  Yes.  I should.

And also return NS_OK; is missing ;)
(In reply to Honza Bambas (:mayhemer) from comment #8)
> (In reply to Michal Novotny (:michal) from comment #7)
> > Comment on attachment 621038 [details] [diff] [review]
> > v1
> > 
> > >    const unsigned long STATE_ITEMCOMPLETED = 7;
> > > +  const unsigned long STATE_ITEMPROGRESS = 11;
> > >    const unsigned long STATE_FINISHED = 10;
> > 
> > Why did you choose 11?

It seems to me that 8 and 9 were never used (http://hg.mozilla.org/mozilla-central/rev/c73c0da830fe), but I'm fine with 11.
(Assignee)

Comment 10

5 years ago
Created attachment 623726 [details] [diff] [review]
v1.1
Attachment #621038 - Attachment is obsolete: true
Attachment #621038 - Flags: review?(michal.novotny)
Attachment #623726 - Flags: review?(michal.novotny)
Attachment #623726 - Flags: review?(michal.novotny) → review+
Honza, is this ready to land? If so, can you land this, or let me know to find someone else to do so? Thanks!
(Assignee)

Comment 12

5 years ago
Comment on attachment 623726 [details] [diff] [review]
v1.1

https://hg.mozilla.org/integration/mozilla-inbound/rev/10b04214089d
Attachment #623726 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/10b04214089d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
No longer blocks: 744712
You need to log in before you can comment on or make changes to this bug.