This is an issue I found while going through this code and
writing/debugging a test for the bug at hand. Without this, the test in
the actual fix for this bug will fail to actually reuse the preloaded
stylesheet.
It seems reasonable to assume that the intersection of quirks mode
documents using link preload headers is small (and in that case we'd
parse the sheet twice, but oh well).
Differential Revision: https://phabricator.services.mozilla.com/D103567
This matches other browsers, and common sense to some extent.
The code is a bit awkward because I want this behind a pref for now, as
it's not precisely a zero-risk change.
Differential Revision: https://phabricator.services.mozilla.com/D95065
This matches other browsers, and common sense to some extent.
The code is a bit awkward because I want this behind a pref for now, as
it's not precisely a zero-risk change.
Differential Revision: https://phabricator.services.mozilla.com/D95065
This matches other browsers, and common sense to some extent.
The code is a bit awkward because I want this behind a pref for now, as
it's not precisely a zero-risk change.
Differential Revision: https://phabricator.services.mozilla.com/D95065
If two loading documents hit the sheet cache and we coalesce the
resource load, there's nothing that prevents the load event on the
second document from firing right now, and there should be.
While at it, also fix the handling of the pending load count, though
it has no correctness impact on the particular test we're fixing here...
We were never decrementing it, which is of course wrong. However it
kinda ended up working because it just causes us to not defer more
loads.
The new assertions and responsibility of the counter should ensure it
stays correct.
Differential Revision: https://phabricator.services.mozilla.com/D80583
This shouldn't have any behavior change, but it makes the code make a
bit more sense. Rather than counting inline stylesheets like a pending
load, we won't (but note that any @import inside it will).
The SheetLoadData::mURL it's supposed to be the url of the stylesheet,
so for StyleSheet::Replace it should be null.
Differential Revision: https://phabricator.services.mozilla.com/D80379
This makes it easy to see why your test is not failing without your
patch, for example ;)
Note that we log only when the URIs are the same, which
I think is a reasonable compromise in verbosity.
Differential Revision: https://phabricator.services.mozilla.com/D80288
This basically undoes D77842, but it was better done on top than just
removing the patch from the stack. I could squash them if desired.
The previous patch to respect caching headers makes tests much much more
happy, to the point where I'm not sure whether we really need this or
not. Your call whether we should keep it or not.
Differential Revision: https://phabricator.services.mozilla.com/D78660
Make the stylesheet cache respect the same headers as the image cache
does. This makes no-cache stylesheets work as they do now, which is
useful for developers that want to develop sites locally, and for
shift-reloads, etc.
Differential Revision: https://phabricator.services.mozilla.com/D78659
This patch implements a per-process cache of parsed stylesheets for
non-inline sheets. The entries are evicted when the document gets
destroyed and there's no other document with the same principal around.
This works fine in practice even when navigating because CC happens
pretty late, but we could add an extra timer if we deem it worth it.
I had to adapt some tests so that they keep passing. They were already
clearing various image / network caches so it seems fine to also clear
this one.
Note that there's a very subtle change in the load data key: We only
miss the cache if the referrer _policy_ is different, not if the
referrer is different. While that is slightly dubious, that is the only
think that makes this effort somewhat worth it. Otherwise stylesheets
would have to be re-fetched if the referrer is different, which
effectively would mean to re-parse it if the document URI is different,
which is bad.
It seems like the network cache only keys on the referrer policy, so it
seems fine to do the same.
Differential Revision: https://phabricator.services.mozilla.com/D77457
This basically undoes D77842, but it was better done on top than just
removing the patch from the stack. I could squash them if desired.
The previous patch to respect caching headers makes tests much much more
happy, to the point where I'm not sure whether we really need this or
not. Your call whether we should keep it or not.
Differential Revision: https://phabricator.services.mozilla.com/D78660
Make the stylesheet cache respect the same headers as the image cache
does. This makes no-cache stylesheets work as they do now, which is
useful for developers that want to develop sites locally, and for
shift-reloads, etc.
Differential Revision: https://phabricator.services.mozilla.com/D78659
This patch implements a per-process cache of parsed stylesheets for
non-inline sheets. The entries are evicted when the document gets
destroyed and there's no other document with the same principal around.
This works fine in practice even when navigating because CC happens
pretty late, but we could add an extra timer if we deem it worth it.
I had to adapt some tests so that they keep passing. They were already
clearing various image / network caches so it seems fine to also clear
this one.
Note that there's a very subtle change in the load data key: We only
miss the cache if the referrer _policy_ is different, not if the
referrer is different. While that is slightly dubious, that is the only
think that makes this effort somewhat worth it. Otherwise stylesheets
would have to be re-fetched if the referrer is different, which
effectively would mean to re-parse it if the document URI is different,
which is bad.
It seems like the network cache only keys on the referrer policy, so it
seems fine to do the same.
Differential Revision: https://phabricator.services.mozilla.com/D77457
I'm about to introduce the concept of "Loader principal" (as in "the
principal of the CSS loader"), and SheetLoadData already has an
mLoaderPrincipal.
However SheetLoadData's principal is just the triggering principal (the
principal that initiated the load). So name it that with consistency
with SheetInfo::mTriggeringPrincipal etc.
Differential Revision: https://phabricator.services.mozilla.com/D77613
The only caller that passes that principal is the only caller that calls
into LoadSheet with a loader that has a document anyway.
And the principal we're passing is Document::NodePrincipal(), which is
what we'd end up using anyway, so the new code is exactly equivalent, as
far as I can tell.
Differential Revision: https://phabricator.services.mozilla.com/D76469
Which is the spec term. nsIStyleSheetLinkingElement is even more
confusing since it may not be an element at all (see: processing
instructions).
Differential Revision: https://phabricator.services.mozilla.com/D76071
When the loader finds an already-complete stylesheet, it will call
PostLoadEvent to fire the load event of the relevant <link> element,
etc.
For that, it'll mint a SheetLoadData, with various fields hard-coded.
That causes us to eventually insert the sheet in mCompleteSheets, but
with a different key, which means we'll end up with two copies of the
same stylesheet in the cache, than when reporting memory we'll report
twice.
Fix it by constructing the right load data a bit sooner, and add an
assertion to ensure this doesn't happen anymore.
Differential Revision: https://phabricator.services.mozilla.com/D76000
This was done by:
This was done by applying:
```
diff --git a/python/mozbuild/mozbuild/code-analysis/mach_commands.py b/python/mozbuild/mozbuild/code-analysis/mach_commands.py
index 789affde7bbf..fe33c4c7d4d1 100644
--- a/python/mozbuild/mozbuild/code-analysis/mach_commands.py
+++ b/python/mozbuild/mozbuild/code-analysis/mach_commands.py
@@ -2007,7 +2007,7 @@ class StaticAnalysis(MachCommandBase):
from subprocess import Popen, PIPE, check_output, CalledProcessError
diff_process = Popen(self._get_clang_format_diff_command(commit), stdout=PIPE)
- args = [sys.executable, clang_format_diff, "-p1", "-binary=%s" % clang_format]
+ args = [sys.executable, clang_format_diff, "-p1", "-binary=%s" % clang_format, '-sort-includes']
if not output_file:
args.append("-i")
```
Then running `./mach clang-format -c <commit-hash>`
Then undoing that patch.
Then running check_spidermonkey_style.py --fixup
Then running `./mach clang-format`
I had to fix four things:
* I needed to move <utility> back down in GuardObjects.h because I was hitting
obscure problems with our system include wrappers like this:
0:03.94 /usr/include/stdlib.h:550:14: error: exception specification in declaration does not match previous declaration
0:03.94 extern void *realloc (void *__ptr, size_t __size)
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/malloc_decls.h:53:1: note: previous declaration is here
0:03.94 MALLOC_DECL(realloc, void*, void*, size_t)
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozilla/mozalloc.h:22:32: note: expanded from macro 'MALLOC_DECL'
0:03.94 MOZ_MEMORY_API return_type name##_impl(__VA_ARGS__);
0:03.94 ^
0:03.94 <scratch space>:178:1: note: expanded from here
0:03.94 realloc_impl
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozmemory_wrap.h:142:41: note: expanded from macro 'realloc_impl'
0:03.94 #define realloc_impl mozmem_malloc_impl(realloc)
Which I really didn't feel like digging into.
* I had to restore the order of TrustOverrideUtils.h and related files in nss
because the .inc files depend on TrustOverrideUtils.h being included earlier.
* I had to add a missing include to RollingNumber.h
* Also had to partially restore include order in JsepSessionImpl.cpp to avoid
some -WError issues due to some static inline functions being defined in a
header but not used in the rest of the compilation unit.
Differential Revision: https://phabricator.services.mozilla.com/D60327
--HG--
extra : moz-landing-system : lando
rg -l 'mozilla/Move.h' | xargs sed -i 's/#include "mozilla\/Move.h"/#include <utility>/g'
Further manual fixups and cleanups to the include order incoming.
Differential Revision: https://phabricator.services.mozilla.com/D60323
--HG--
extra : moz-landing-system : lando
Relatedly: We only use this to determine priority. It seems we prioritize
<link rel=preload> over <link rel=stylesheet>, is that intended?
That seems a bit weird, as the preloads from the parser are likely to be used
very soon.
Differential Revision: https://phabricator.services.mozilla.com/D55225
--HG--
extra : moz-landing-system : lando
ReferrerPolicy gets tossed back and forth as a uint32_t and
ReferrerPolicy enum in header file. Expose ReferrerPolicyValues from
webidl file and use consistently in native code.
Differential Revision: https://phabricator.services.mozilla.com/D41954
--HG--
extra : moz-landing-system : lando
Saving a refcount bump is not worth the churn. Use a proper RefPtr<>
everywhere instead of manual refcounting, and don't make DoSheetComplete call
NS_RELEASE unconditionally.
Also, make clear by using references where null is expected or not.
Also, properly use a RefPtr in mPendingDatas, since they are strong pointers,
in fact.
Finally, remove some unused macros from nsCSSValue of which this code was the
last consumer.
Differential Revision: https://phabricator.services.mozilla.com/D41090
--HG--
extra : moz-landing-system : lando
They're bad, specially if they do vastly different thing in overloaded
functions, like aUseSystemPrincipal and aIsPreload. :)
Differential Revision: https://phabricator.services.mozilla.com/D40851
--HG--
extra : moz-landing-system : lando
Emilio Cobos Álvarez
Narcis Beleuzu
Simon Giesecke
Honza Bambas
Noemi Erli
Razvan Maries
Ting-Yu Lin
Jonathan Kingston
Thomas Nguyen