Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AppCache fix for slow updates #248

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

AppCache fix for slow updates #248

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions tpls/appcache-frame.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,6 @@
clearInterval(downloadingInterval);
downloadingInterval = null;
}

applicationCache.removeEventListener('downloading', onDownloadingEvent);
applicationCache.removeEventListener('progress', onDownloadingEvent);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, problem with removing this is that progress happens multiple times, i.e. it may happen once per file. So removing this cleanup makes onUpdating event fire very often.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'm probably wrong about firing it multiple times because of updatingFired check. But this check will prevent onUpdating event to fire when autoUpdate happens. In other words, it's more complicated than just removing those lines.

Copy link
Author

@jampy jampy May 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it by design (and desired) that progress happens multiple times? Even with the current code those events come in the same way in the first 5 seconds. I don't see the problem, sorry.

What about just keeping the updateready event? After all that's the single most important one to get updates working instantly AFAIK. That would still remain a simple fix that could be released soon.

As for a more complete fix I think it would be best if the event handlers (perhaps those outside of the iframe) can deal with various event "frequencies". After all we learned that browser behaviour is somewhat unpredictable and nobody can guarantee what events come in in what order...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NekR ...?

}

function cleanUp() {
Expand All @@ -103,10 +100,6 @@

downloadingCleanUp();

applicationCache.removeEventListener('updateready', onUpdateReadyEvent);
applicationCache.removeEventListener('cached', onInstalledEvent);
applicationCache.removeEventListener('obsolete', onObsoleteEvent);

if (updateReadyInterval) {
clearInterval(updateReadyInterval);
updateReadyInterval = null;
Expand Down