Enable QM_TRY reporting to the browser console in release builds
Categories
(Core :: Storage: Quota Manager, task, P2)
Tracking
()
People
(Reporter: janv, Assigned: janv)
Details
Attachments
(11 files)
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
I'll provide more details later, I'm currently investigating code size implications.
Assignee | ||
Comment 1•3 years ago
|
||
Assignee | ||
Comment 2•3 years ago
|
||
Depends on D117132
Assignee | ||
Comment 3•3 years ago
|
||
Depends on D117133
Assignee | ||
Comment 4•3 years ago
|
||
Depends on D117134
Assignee | ||
Comment 5•3 years ago
|
||
Depends on D117135
Assignee | ||
Comment 6•3 years ago
|
||
Depends on D117136
Assignee | ||
Comment 7•3 years ago
|
||
Depends on D117137
Assignee | ||
Comment 8•3 years ago
|
||
Depends on D117138
Assignee | ||
Comment 9•3 years ago
|
||
This patch doesn't enable or disable logging in specific builds.
Depends on D117139
Assignee | ||
Comment 10•3 years ago
|
||
Failures are not reported to the browser console until a context information is
available like the storage or temporary storage initialization.
Depends on D117140
Assignee | ||
Comment 11•3 years ago
|
||
MOZ_NEVER_INLINE can't 100% guarantee that all compilers never inline such
functions, but the current combination of MOZ_COLD and MOZ_NEVER_INLINE
produces sane builds with only minor code size increase (max 0.19% increase).
Depends on D117141
Assignee | ||
Comment 12•3 years ago
|
||
Comment 13•3 years ago
|
||
Updated•3 years ago
|
Comment 14•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e96eff0eb8ea
https://hg.mozilla.org/mozilla-central/rev/be98873b8404
https://hg.mozilla.org/mozilla-central/rev/ec6e82c8b989
https://hg.mozilla.org/mozilla-central/rev/0f2b472fe633
https://hg.mozilla.org/mozilla-central/rev/a7d288075734
https://hg.mozilla.org/mozilla-central/rev/3ac5bb33ec5d
https://hg.mozilla.org/mozilla-central/rev/97747f7a58cc
https://hg.mozilla.org/mozilla-central/rev/5ba6c93ff181
https://hg.mozilla.org/mozilla-central/rev/1ba47f35b50d
https://hg.mozilla.org/mozilla-central/rev/17e9bc4f9bc7
https://hg.mozilla.org/mozilla-central/rev/ae93a7d0e2cc
Assignee | ||
Comment 15•3 years ago
|
||
Comment on attachment 9225849 [details]
Bug 1713820 - Enable QM_TRY reporting to the browser console in release builds; r=#dom-storage
Beta/Release Uplift Approval Request
- User impact if declined: localStorge next-gen (LSNG) is going to be enabled in 90 release for some users (as part of a staged rollout). It will be much easier to identify possible problems reported by users if QM_TRY logging to the browser console is enabled in 90 too.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Code size implications have been analyzed for all platforms and it turns out that the increase is only +0.19% (size of lib XUL) in the worst case. Possible spamming of the browser console shouldn't occur since the logging is currently limited only to specific contexts (storage and temporary storage initialization) and only enabled during the first initialization attempt. There are multiple patches for this, but they are all quite simple. The logging to the browser console have been enabled in early beta or earlier for long time. All in all, these pathces should be quite safe for an uplift.
- String changes made/needed: None
Assignee | ||
Updated•3 years ago
|
Comment 16•3 years ago
|
||
Comment on attachment 9225840 [details]
Bug 1713820 - Add dedicated QM_LOG_ERROR_ENABLED identifier for conditional LogError compilation; r=#dom-storage
approved for 90.0b7, thanks
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 17•3 years ago
|
||
Comment on attachment 9225847 [details]
Bug 1713820 - Don't report to the browser console if the context string is empty; r=#dom-storage
so many patches :)
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 18•3 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #17)
so many patches :)
But they are small and comprehensive :) !
Comment 19•3 years ago
|
||
Yep, definitely.
Comment 20•3 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/1da51f20f721
https://hg.mozilla.org/releases/mozilla-beta/rev/08201e5b65bb
https://hg.mozilla.org/releases/mozilla-beta/rev/2dd7d08a49c4
https://hg.mozilla.org/releases/mozilla-beta/rev/09014bdb8693
https://hg.mozilla.org/releases/mozilla-beta/rev/d67ca06c85d9
https://hg.mozilla.org/releases/mozilla-beta/rev/4950c5144857
https://hg.mozilla.org/releases/mozilla-beta/rev/51c42176714e
https://hg.mozilla.org/releases/mozilla-beta/rev/1fb0c59d64b4
https://hg.mozilla.org/releases/mozilla-beta/rev/20f24febcc54
https://hg.mozilla.org/releases/mozilla-beta/rev/b295b08ee150
https://hg.mozilla.org/releases/mozilla-beta/rev/307025cdae1d
Description
•