1525291 - LSNG: Chrome observer notifications for session storage are not distributed to content processes

Status: RESOLVED FIXED

(Core :: Storage: localStorage & sessionStorage, defect)

Description

[Jan Varga [:janv]]

This bug existed before, but wasn't visible for some reason.
However, after bug 1483440, this test started to fail with LSNG enabled:
browser/components/sessionstore/test/browser_sessionStorage.js

The problem is, that the PBackgroundStorage, which is also used for distributing notifications to content processes, is not constructed.

I have a patch that adds PSessionStorageObserver protocol when LSNG is enabled.

Comment 1

[Jan Varga [:janv]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bcb30bc8d48d2493532667864c81756c8cc899b

Comment 2

[Jan Varga [:janv]]

Attachment: Bug 1525291 - LSNG: Chrome observer notifications for session storage are not distributed to content processes; r=asuth

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Okay, so restating the context of this bug as I understand it after some investigation:

• Bug 1483440 changed the status quo implementation-wise:
  • The observer notification "browser:purge-domain-data" and the StorageObserver internal "domain-data-cleared" notification were removed. It cleared both sessionStorage and localStorage (and session history things).
  • Storage-wise, it was replaced with separate "browser:purge-sessionStorage" and "extension:purge-localStorage" notifications. (There is no meaningful difference between the "browser" and "extension" prefixes... the "extension" one started out only being emitted by the WebExtensions API in ext-browsingData.js, but now is used by browsers.)
• Bug 1483440 also changed the browser_sessionStorage.js test to be more realistic, which is what has revealed the regression:
  • Previously, code at https://hg.mozilla.org/mozilla-central/rev/170f979a5f7d#l6.12 was broadcasting a message that would be received at https://hg.mozilla.org/mozilla-central/rev/170f979a5f7d#l7.12 and would generate a "browser:purge-domain-data" observer notification in the child.
  • The code was changed to use the ClearDataService at https://hg.mozilla.org/mozilla-central/rev/170f979a5f7d#l6.14 which is ideal because that's how data gets cleared in actual use.
• So as :janv says in comment 0, the problem already existed with LSNG, the test had just been covering up the problem. (And much of this was due to the historically hacky manner in which data clearing was implemented. We've come a long way thanks to :johannh and :baku!)
• In legacy localStorage, notifications were being propagated from the parent process to the child process via StorageDBParent/StorageDBChild (which involved thread bouncing after the move to PBackgroundin bug 1350637). Invariant-wise, there was only one StorageDBChild per process, eagerly created by LocalStorageManager.
• So SessionStorage was basically free-riding on the LocalStorage implementation, although this wasn't entirely obvious. (The SessionStorage/LocalStorage split was not entirely complete.)

And so onto the review!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/04eecadabb48
LSNG: Chrome observer notifications for session storage are not distributed to content processes; r=asuth

Comment 5

[Oana Pop-Rus]

Backed out changeset 04eecadabb48 (bug 1525291) for build bustages at build/src/obj-firefox/ipc/ipdl/PSessionStorageObserverChild.cpp

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/a83047aca8846cbba0324c38ee2c35806746915d

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=superseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&selectedJob=226760944&revision=04eecadabb485bd398c6c37e258ba47b946e7982

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=226760906&repo=mozilla-inbound&lineNumber=14532

[task 2019-02-07T09:19:43.098Z] 09:19:43 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl'
[task 2019-02-07T09:19:43.098Z] 09:19:43 INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ --target=i686-linux-gnu -o UnifiedProtocols26.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/ipc/ipdl -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -I/builds/worker/workspace/build/src/widget/gtk/compat-gtk3 -pthread -I/usr/include/gtk-3.0 -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/pixman-1 -I/usr/include/libpng12 -I/usr/include/gtk-3.0/unix-print -MD -MP -MF .deps/UnifiedProtocols26.o.pp /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/UnifiedProtocols26.cpp
[task 2019-02-07T09:19:43.099Z] 09:19:43 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/UnifiedProtocols26.cpp:83:
[task 2019-02-07T09:19:43.099Z] 09:19:43 INFO - /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PSessionStorageObserverChild.cpp:8:10: fatal error: 'mozilla/dom/SessionStorageObserverChild.h' file not found
[task 2019-02-07T09:19:43.099Z] 09:19:43 INFO - #include "mozilla/dom/SessionStorageObserverChild.h"
[task 2019-02-07T09:19:43.099Z] 09:19:43 INFO - ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2019-02-07T09:19:43.099Z] 09:19:43 INFO - 1 error generated.
[task 2019-02-07T09:19:43.099Z] 09:19:43 INFO - /builds/worker/workspace/build/src/config/rules.mk:812: recipe for target 'UnifiedProtocols26.o' failed
[task 2019-02-07T09:19:43.099Z] 09:19:43 ERROR - make[4]: *** [UnifiedProtocols26.o] Error 1
[task 2019-02-07T09:19:43.099Z] 09:19:43 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl'
[task 2019-02-07T09:19:43.100Z] 09:19:43 INFO - make[4]: *** Waiting for unfinished jobs....
[task 2019-02-07T09:19:43.100Z] 09:19:43 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/media/webrtc/signaling/gtest'
[task 2019-02-07T09:19:43.100Z] 09:19:43 INFO - media/webrtc/signaling/gtest/rtpsources_unittests.o

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/87f055bd4551
LSNG: Chrome observer notifications for session storage are not distributed to content processes; r=asuth,froydnj

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/87f055bd4551