Honza Bambas (:mayhemer) Reporter
	Description • 9 years ago

First, this notification is under-documented.  It's not clear what the 'aData' argument is carrying and how exactly to handle it.  

So, to sum, AFAIK (from Yoshi and Bobby Holley) the correct implementation should be based on the following:
- aData is a JSON that should be fed to OriginAttributesPattern class (Init() method)
- when OriginAttributesPattern::Init() fails, observers should throw
- checking then has to "match" as |pattern.Matches(myDataOriginAttributes)|, where |pattern| is an Init(aData)'ed OriginAttributesPattern object ; iff that returns true, you have to delete the data
- note, that when the aData JSON string is empty the result is a pattern object whose Matches() method always returns true, so, all data has to be deleted


Second, checking on already existing implementers reveals not everyone doing it right.

- HTTP cache (CacheObserver.cpp), which doesn't do the right thing at all (already has a bug 1214360 filed to fix it) 
- PushService [1], bug 1170115, that seems to do the right thing, except that empty aData string is simply ignored (the code doesn't delete anything)
- ServiceWorkerManager [2], bug 1191647, that doesn't build OriginAttributesPattern from aData
- nsCookieService [3], bug 1165267, similar problem

There are just two consumers doing the right thing: nsPermissionManager [4] and nsHttpAuthCache [5]


Outcome of this bug should be:
- document the notification properly so that it can be easily found on MDC, dev.platform post would be good too
- file bugs to fix the currently wrongly implemented handlers (or state clearly those are correct in the current form)


[1] http://hg.mozilla.org/mozilla-central/annotate/0babaa3edcf9/dom/push/PushService.jsm#l280
[2] http://hg.mozilla.org/mozilla-central/annotate/0babaa3edcf9/dom/workers/ServiceWorkerManager.cpp#l4607
[3] http://hg.mozilla.org/mozilla-central/annotate/0babaa3edcf9/netwerk/cookie/nsCookieService.cpp#l573

[4] http://hg.mozilla.org/mozilla-central/annotate/0babaa3edcf9/extensions/cookie/nsPermissionManager.cpp#l2304
[5] http://hg.mozilla.org/mozilla-central/annotate/0babaa3edcf9/netwerk/protocol/http/nsHttpAuthCache.cpp#l268

	Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]
	Comment 1 • 9 years ago

(In reply to Honza Bambas (:mayhemer) from comment #0)
> - note, that when the aData JSON string is empty the result is a pattern object whose > Matches() method always returns true, so, all data has to be deleted
...
> - PushService [1], bug 1170115, that seems to do the right thing, except
> that empty aData string is simply ignored (the code doesn't delete anything)

Right now Webapps.jsm doesn't notify "" in clear-origin-data, also we didn't define how to delete all data yet, although right now "{}" will delete all.

So far I'd say PushService is ok and we might need another bug to discuss whether we need 'delete all' or not.

	Sylvestre Ledru [:Sylvestre]
	Comment 2 • 6 years ago

Core Graveyard / DOM: Apps is inactive. Closing all bugs in this component.