Re: [whatwg/storage] Add UsageDetails dictionary (#69)

domenic commented on this pull request.

This overall architecture makes sense to me now (although @annevk should check, as the editor). What remains are editorial issues.

> @@ -178,7 +178,18 @@ larger <a>site storage quota</a>. Factors such as navigation frequency, recency
 bookmarking, and <a href="#persistence">permission</a> for {{"persistent-storage"}} can be used as
 indications of "popularity".
 
+The <dfn export>application cache site storage usage</dfn> for an origin is a rough estimate of the amount

Nit in these definitions: _origin_ is used but not defined. Either say "an origin _origin_" or later say "the origin's site storage unit"

> @@ -178,7 +178,18 @@ larger <a>site storage quota</a>. Factors such as navigation frequency, recency
 bookmarking, and <a href="#persistence">permission</a> for {{"persistent-storage"}} can be used as
 indications of "popularity".
 
+The <dfn export>application cache site storage usage</dfn> for an origin is a rough estimate of the amount
+of bytes used in Application Cache in <var>origin</var>'s <a>site storage unit</a>. [[HTML#appcache]]

I think these should be normative references, `[[!`, not informative ones, `[[`

>  
+The <dfn export>caches site storage usage</dfn> for an origin is a rough estimate of the amount
+of bytes used in Cache API in <var>origin</var>'s <a>site storage unit</a>. [[service-workers-1#cache-objects]]

Reference `[[SERVICE-WORKERS]]`, not `[[SERVICE-WORKERS-1]]`, if possible. (It might not be; there have been weird bugs in the past.)

> +   <li><p>Let <var>applicationCache</var> be <a>application cache site storage usage</a>
+   for <var>origin</var>.
+
+   <li><p>Let <var>indexedDB</var> be <a>indexedDB site storage usage</a> for <var>origin</var>.
+
+   <li><p>Let <var>caches</var> be <a>caches site storage usage</a> for <var>origin</var>.
+
+   <li><p>Let <var>serviceWorkerRegistrations</var> be <a>service worker registration
+   site storage usage</a> for <var>origin</var>.
+
+   <li><p>Let <var>usageDetails</var> be a new {{StorageUsageDetails}} dictionary
+   whose {{StorageUsageDetails/applicationCache}} member is <var>applicationCache</var>,
+   {{StorageUsageDetails/indexedDB}} member is <var>indexedDB</var>, {{StorageUsageDetails/caches}}
+   member is <var>caches</var>, and {{StorageUsageDetails/serviceWorkerRegistrations}} member is
+   <var>serviceWorkerRegistrations</var>. Any member for which the <a>site storage usage</a>
+   is less than 0 should be excluded from the {{StorageUsageDetails}} dictionary.

How could they be less than 0?

> @@ -331,8 +351,26 @@ must run these steps:
 
    <li><p>Let <var>quota</var> be <a>site storage quota</a> for <var>origin</var>.
 
-   <li><p>Let <var>dictionary</var> be a new {{StorageEstimate}} dictionary whose {{usage}} member
-   is <var>usage</var> and {{quota}} member is <var>quota</var>.
+   <li><p>Let <var>applicationCache</var> be <a>application cache site storage usage</a>
+   for <var>origin</var>.
+
+   <li><p>Let <var>indexedDB</var> be <a>indexedDB site storage usage</a> for <var>origin</var>.
+
+   <li><p>Let <var>caches</var> be <a>caches site storage usage</a> for <var>origin</var>.
+
+   <li><p>Let <var>serviceWorkerRegistrations</var> be <a>service worker registration
+   site storage usage</a> for <var>origin</var>.
+
+   <li><p>Let <var>usageDetails</var> be a new {{StorageUsageDetails}} dictionary

This step is weird because you say to create the dictionary with all these things set, but then you say "actually, exclude them sometimes".

It would be better to phrase this informatively. Roughly,

1. Let _usageDetails_ be a new `StorageUsageDetails` dictionary.
1. If _applicationCache_ > 0, then set _usageDetails_["`applicationCache`"] to _applicationCache_.
1. If...

> +
+   <li><p>Let <var>caches</var> be <a>caches site storage usage</a> for <var>origin</var>.
+
+   <li><p>Let <var>serviceWorkerRegistrations</var> be <a>service worker registration
+   site storage usage</a> for <var>origin</var>.
+
+   <li><p>Let <var>usageDetails</var> be a new {{StorageUsageDetails}} dictionary
+   whose {{StorageUsageDetails/applicationCache}} member is <var>applicationCache</var>,
+   {{StorageUsageDetails/indexedDB}} member is <var>indexedDB</var>, {{StorageUsageDetails/caches}}
+   member is <var>caches</var>, and {{StorageUsageDetails/serviceWorkerRegistrations}} member is
+   <var>serviceWorkerRegistrations</var>. Any member for which the <a>site storage usage</a>
+   is less than 0 should be excluded from the {{StorageUsageDetails}} dictionary.
+
+   <li><p>Let <var>dictionary</var> be a new {{StorageEstimate}} dictionary whose {{StorageEstimate/usage}} member
+   is <var>usage</var>, {{StorageEstimate/quota}} member is <var>quota</var> and {{StorageEstimate/usageDetails}}
+   member is <var>usageDetails</var>.
 
    <li>
     <p>If there was an internal error while obtaining <var>usage</var> and <var>quota</var>, then

Maybe this should be changed to "obtaining any of the above"?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/storage/pull/69#pullrequestreview-244353118