Implement Storage::SupportedPropertyNames

[ddrmanxbxfr]

Hi guys,

This is a rough draft for issue #7670 .

It includes :

• SupportedPropertyNames implementation
• WPT with Object.supportedpropertynames with Local Storage and Session Storage.

The following link help me understood the issue :
http://www.w3.org/TR/webstorage/#the-storage-interface
https://html.spec.whatwg.org/multipage/infrastructure.html#supported-property-names

Thanks for looking into it.

[nox]

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{components/net/storage_task.rs, line 113 [r1] (raw file):}
Why do you send Option<Vec<DOMString>> and not just a Vec<DOMString>?

---

Comments from the review on Reviewable.io

[ddrmanxbxfr]

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{components/net/storage_task.rs, line 113 [r1] (raw file):}
I wanted to stay consistent with the current interface in storage task.

All the empty elements are handled using Options types instead of casting them to empty string or vec![].

It kinda make sense as we have pattern matching available in rust and it's explicit.

---

Comments from the review on Reviewable.io

[Ms2ger]

Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

---

^{components/net/storage_task.rs, line 122 [r1] (raw file):}
Align the .unwrap() with .send(

---

Comments from the review on Reviewable.io

[nox]

-S-awaiting-answer +S-needs-code-changes

---

Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

---

^{components/net/storage_task.rs, line 113 [r1] (raw file):}
I'm not convinced, see length() two methods above.

---

Comments from the review on Reviewable.io

[ddrmanxbxfr]

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{components/net/storage_task.rs, line 113 [r1] (raw file):}
https://github.com/ddrmanxbxfr/servo/blob/StorageSupportedAttributes/components/net_traits/storage_task.rs#L22

Length indeed provides a default value. However, key, setitem, getitem, removeitem all provides Option response as empty elements.

If we move to a Vec when we have a empty element from IPCSender shall we send vec![] as default value ?

If yes, i'll unwrap the ipc object and provide a default value.

Thanks.

---

Comments from the review on Reviewable.io

[nox]

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{components/net/storage_task.rs, line 113 [r1] (raw file):}
vec![] doesn't allocate anything, so you can just use IpcSender<Vec<DOMString>> and it will be fine.

---

Comments from the review on Reviewable.io

[ddrmanxbxfr]

Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

^{components/net/storage_task.rs, line 113 [r1] (raw file):}
It's cool, removing the option thing :).

---

Comments from the review on Reviewable.io

[ddrmanxbxfr]

@nox ping?

[nox]

-S-awaiting-review +S-needs-code-changes

---

Reviewed 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

---

^{components/net/storage_task.rs, line 122 [r2] (raw file):}
You shouldn't be unwrapping here, but using:

data.get(&origin).map_or(vec![], |entry| entry.keys().cloned().collect()))

See the similar piece of code in length().

---

Comments from the review on Reviewable.io

[ddrmanxbxfr]

Review status: 4 of 5 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{components/net/storage_task.rs, line 122 [r2] (raw file):}
Indeed. Avoids to panic. Fixed.

---

Comments from the review on Reviewable.io

[nox]

@bors-servo r+

---

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

Comments from the review on Reviewable.io

[bors-servo]

📌 Commit 080f76a has been approved by nox

[bors-servo]

⌛ Testing commit 080f76a with merge 21658f8...

[bors-servo]

💔 Test failed - mac-dev-ref-unit

[ddrmanxbxfr]

I'll rebase against master and see if test ref still fails on my desktop.

[jdm]

@bors-servo: r+
Don't worry, that was just #7730.

[bors-servo]

📌 Commit e7a3220 has been approved by jdm

[bors-servo]

⌛ Testing commit e7a3220 with merge 02da3a1...

[bors-servo]

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt