1204581 - Add a deprecation notice for AppCache if service worker fetch interception is enabled

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[(no longer active)]

Attachment: Screenshot

It's time to start warn people to stop using AppCache.

Comment 1

[(no longer active)]

Attachment: Add a deprecation warning for the usage of AppCache when service worker fetch interception is enabled

Requesting review from Jason on the Necko bits and baku on the rest.

Comment 2

[Andrea Marchesini [:baku]]

Comment on attachment 8660829 [details] [diff] [review]
Add a deprecation warning for the usage of AppCache when service worker fetch interception is enabled

Review of attachment 8660829 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm!

::: netwerk/protocol/http/nsHttpChannel.cpp
@@ +7032,5 @@
> +                          true);
> +
> +    // Then, issue a deprecation warning if service worker interception is
> +    // enabled.
> +    if (Preferences::GetBool("dom.serviceWorkers.interception.enabled", false)) {

I have to remember to remove this when patch for bug 1163545 will land. I added a nice:

nsContentUtils::ServiceWorkerInterceptionEnabled()

::: netwerk/protocol/http/nsHttpChannel.h
@@ +397,5 @@
>                                    bool checkingAppCacheEntry);
>  
>      void SetPushedStream(Http2PushedStream *stream);
>  
> +    void MaybeWarnAboutAppCache();

maybe this can be const?

Comment 3

[(no longer active)]

(In reply to Andrea Marchesini (:baku) from comment #2)
> ::: netwerk/protocol/http/nsHttpChannel.cpp
> @@ +7032,5 @@
> > +                          true);
> > +
> > +    // Then, issue a deprecation warning if service worker interception is
> > +    // enabled.
> > +    if (Preferences::GetBool("dom.serviceWorkers.interception.enabled", false)) {
> 
> I have to remember to remove this when patch for bug 1163545 will land. I
> added a nice:
> 
> nsContentUtils::ServiceWorkerInterceptionEnabled()

Nice!

> ::: netwerk/protocol/http/nsHttpChannel.h
> @@ +397,5 @@
> >                                    bool checkingAppCacheEntry);
> >  
> >      void SetPushedStream(Http2PushedStream *stream);
> >  
> > +    void MaybeWarnAboutAppCache();
> 
> maybe this can be const?

It can't, because NS_QueryNotificationCallbacks() requires non-const loadgroup and callbacks arguments.

Comment 4

[(no longer active)]

Attachment: Add a deprecation warning for the usage of AppCache when service worker fetch interception is enabled

The previous patch mistakenly called MaybeWarnAboutAppCache() from OnNormalCacheEntryAvailable() too.

Comment 5

[(no longer active)]

Comment on attachment 8660854 [details] [diff] [review]
Add a deprecation warning for the usage of AppCache when service worker fetch interception is enabled

Ping?  I was hoping to land this before the uplift and I lost track of it, and now we cannot uplift because of the string changes.  :(

Comment 6

[Patrick McManus [:mcmanus]]

Comment on attachment 8660854 [details] [diff] [review]
Add a deprecation warning for the usage of AppCache when service worker fetch interception is enabled

Review of attachment 8660854 [details] [diff] [review]:
-----------------------------------------------------------------

r+ on netwerk

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/19400ffcf469

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/19400ffcf469

Comment 9

[u232883]

Added the site compatibility doc: https://www.fxsitecompat.com/en-US/docs/2015/application-cache-api-has-been-deprecated/

Comment 10

[(no longer active)]

(In reply to Kohei Yoshino [:kohei] from comment #9)
> Added the site compatibility doc:
> https://www.fxsitecompat.com/en-US/docs/2015/application-cache-api-has-been-
> deprecated/

Do you mind fixing the page please?  We have not shipped service workers yet, and our implementation of Cache API without service workers is not quite useful for replacing appcache yet.

Comment 11

[u232883]

Gah, I thought at least the Cache API had already been shipped. Fixed the copy accordingly. Thanks for flagging!

https://github.com/fxsitecompat/www.fxsitecompat.com/commit/83e9986

Comment 12

[Chris Mills [:cmills]]

I have added a note covering this in appropriate places on 

https://developer.mozilla.org/en-US/Firefox/Releases/44#Service_Workers

https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API/Using_Service_Workers

https://developer.mozilla.org/en-US/docs/Web/HTML/Using_the_application_cache

Let me know if that's ok, or if anything more is needed.

Comment 13

[Matt Falkenhagen]

(In reply to :Ehsan Akhgari from comment #10)
> (In reply to Kohei Yoshino [:kohei] from comment #9)
> > Added the site compatibility doc:
> > https://www.fxsitecompat.com/en-US/docs/2015/application-cache-api-has-been-
> > deprecated/
> 
> Do you mind fixing the page please?  We have not shipped service workers
> yet, and our implementation of Cache API without service workers is not
> quite useful for replacing appcache yet.

Should <https://www.fxsitecompat.com/en-CA/docs/2015/application-cache-api-has-been-deprecated/> be updated now to say service workers shipped in Firefox 44, rather than "available on Firefox Developer Edition for testing"?

Comment 14

[(no longer active)]

(In reply to Matt Falkenhagen from comment #13)
> (In reply to :Ehsan Akhgari from comment #10)
> > (In reply to Kohei Yoshino [:kohei] from comment #9)
> > > Added the site compatibility doc:
> > > https://www.fxsitecompat.com/en-US/docs/2015/application-cache-api-has-been-
> > > deprecated/
> > 
> > Do you mind fixing the page please?  We have not shipped service workers
> > yet, and our implementation of Cache API without service workers is not
> > quite useful for replacing appcache yet.
> 
> Should
> <https://www.fxsitecompat.com/en-CA/docs/2015/application-cache-api-has-been-
> deprecated/> be updated now to say service workers shipped in Firefox 44,
> rather than "available on Firefox Developer Edition for testing"?

Yes, good catch!  Kohei, do you mind updating that site, please?  Thanks!

Comment 15

[u232883]

Thanks, updated the compat doc.