1295761 - After rapidly adding and removing a localStorage item, it's still shown in Storage Inspector

Status: RESOLVED WORKSFORME

(DevTools :: Storage Inspector, defect, P2)

Description

[Stephanie Hobson [:shobson]]

Okay, this is a weird one but I don't *think* it's my code...

1) visit https://developer.mozilla.org/en-US/
2) open storage inspector in dev tools and select local storage and then https://developer.mozilla.org/en-US/
3) select "CSS" out of "Web Platform" menu
4) watch two keys appear in LocalStorage, one is 'test' and the other is the current date and time
 
Reasons I don't think this is my code:
- my code does not include the string 'test' in it anywhere
- it only happens in FireFox, not Chrome
- closing and reopening the dev tools makes them disappear (which is not true of keys with empty values when I set them myself)

Reasons why it's probably my code:
- can't reproduce anywhere else
- there are more people QAing Firefox than there are MDN ;)
- Chrome doesn't let you keep their dev tools open and observe the keys when refreshing pages, it's possible it's being written and removed in Chrome because I can observe it

Tried it in Dev Edition which I assume is a clean profile, same result.

Comment 1

[Stephanie Hobson [:shobson]]

I should add: 

- no plug-ins in Dev Edition, it dumps everything when I logout
- turned my adblocker paranoia settings up to high and it still happened so I don't think it's a 3rd party script.

Comment 2

[Ciprian Muresan [:cmuresan], Ecosystem QA]

Hi Stephanie,

I've managed to reproduce this issue on the latest Nightly.
The following pushlog introduced the issue: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e847cfcb315f511f4928b03fd47dcf57aad05e1e&tochange=1801b99994e4f29cd49fa34d840f0c40ba8be304

Jarda, could you take a look at this?

Comment 3

[Jarda Snajdr [:jsnajdr]]

I can reproduce it, too. I'll let you know when I figure out something.

Comment 4

[Jarda Snajdr [:jsnajdr]]

"test" is written here:
https://login.persona.org/include.js:8:11074

a.localStorage.setItem("test","true"),a.localStorage.removeItem("test")

It should be immediately removed though. It's strange that Storage Inspector keeps showing it. Looks like a bug.

The date value is written here:
https://developer.cdn.mozilla.net/static/build/js/main.a7b27a25fb9c.js:5:18125

localStorage.setItem(uid,uid);result=localStorage.getItem(uid)==uid;localStorage.removeItem(uid)

Again, the storage item should have been immediately removed. Instead, it's still shown in the Storage Inspector, although the value is empty.

It would be nice if devtools enabled developers to find these code locations, wouldn't it? I had to add some C++ tweaks in the DOM storage code to get them.

Comment 5

[Stephanie Hobson [:shobson]]

Ah, it's Persona! That makes sense. It wouldn't have been blocked by my very paranoid ad blockers.

We can close this if you like, or move it over to the Storage Inspector component.

Comment 6

[Jarda Snajdr [:jsnajdr]]

The localStorage item fails to be removed from the Storage Inspector when removed too quickly. Renaming the bug.

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

STR:
localStorage.setItem("test","true")
localStorage.removeItem("test")

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Filter on HOTFROG.

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Bug 1295761 - Stop storage inspector showing items that have been quickly added & removed

Changes:

- Fixed the main issue where the delete event must be processed **after** added or changed events because they arrive in batches of changes.
- Fixed a bunch of errors where we called async functions without yielding leading to multiple possible race conditions.
- In one place a method was async but contained no yield so this has been changed to a normal function.

Review commit: https://reviewboard.mozilla.org/r/151320/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151320/

Comment 10

[Patrick Brosset <:pbro>]

Comment on attachment 8879945 [details]
Bug 1295761 - Stop storage inspector showing items that have been quickly added & removed

https://reviewboard.mozilla.org/r/151320/#review157048

This change feels good. Thanks!

Comment 11

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 7b75948abbfb -d 895701cff94e: rebasing 403625:7b75948abbfb "Bug 1295761 - Stop storage inspector showing items that have been quickly added & removed r=pbro" (tip)
merging devtools/client/storage/ui.js
warning: conflicts while merging devtools/client/storage/ui.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 12

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8879945 [details]
Bug 1295761 - Stop storage inspector showing items that have been quickly added & removed

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151320/diff/1-2/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d0c5443a348
Stop storage inspector showing items that have been quickly added & removed r=pbro

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7d0c5443a348

Comment 15

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/mozilla-central/rev/a584b41ad399 for... being the thing I could find to back out to stop the permaorange of bug 1376281. Since this was green when it merged to mozilla-central, by rights whatever thing in https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=af9a8cebd43fb23822be25c5d4d065d5e9e195b8 it was that disagreed with it should have been backed out instead, but I couldn't spot anything in there that seemed enough like a likely candidate to be worth pushing a backout to try.

Comment 18

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

This was fixed by bug 1443461.