Closed Bug 1525291 Opened 6 years ago Closed 6 years ago

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

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: janv, Assigned: janv)

References

Details

Attachments

(1 file)

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.

Blocks: 1517090
Assignee: nobody → jvarga
Status: NEW → ASSIGNED
Attachment #9041725 - Attachment description: imported patch ss-observer → Bug 1525291 - LSNG: Chrome observer notifications for session storage are not distributed to content processes; r=asuth

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:
  • 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!

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

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

Flags: needinfo?(jvarga)
Pushed by jvarga@mozilla.com: 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
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Flags: needinfo?(jvarga)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: