Round of CSS loader cleanup.
Categories
(Core :: CSS Parsing and Computation, task, P3)
Tracking
()
People
(Reporter: emilio, Assigned: emilio)
References
Details
Attachments
(18 files, 1 obsolete file)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
Assignee | ||
Comment 1•4 years ago
|
||
I'm going to tweak it a bit.
Assignee | ||
Comment 2•4 years ago
|
||
Little pet peeve o' mine.
Depends on D40687
Assignee | ||
Comment 3•4 years ago
|
||
Depends on D40688
Assignee | ||
Comment 4•4 years ago
|
||
Now that Sheets is out of line this can move to the cpp file too.
Depends on D40689
Assignee | ||
Comment 5•4 years ago
|
||
Rather than checking after-the fact and dropping the cached result on the floor.
Depends on D40690
Assignee | ||
Comment 6•4 years ago
|
||
Just moving code around but hopefully the new code is nicer :)
Depends on D40691
Assignee | ||
Comment 7•4 years ago
|
||
Also, it's infallible, so no need to have an error state.
Depends on D40692
Updated•4 years ago
|
Assignee | ||
Comment 8•4 years ago
|
||
It doesn't do much for them.
Assignee | ||
Comment 9•4 years ago
|
||
Assignee | ||
Comment 10•4 years ago
|
||
Assignee | ||
Comment 11•4 years ago
|
||
On the fence on this one, but I do think it's nicer.
Assignee | ||
Comment 12•4 years ago
|
||
They're bad, specially if they do vastly different thing in overloaded
functions, like aUseSystemPrincipal and aIsPreload. :)
Assignee | ||
Comment 13•4 years ago
|
||
It's a refptr hashtable, really.
Assignee | ||
Comment 14•4 years ago
|
||
Assignee | ||
Comment 15•4 years ago
|
||
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.
Assignee | ||
Comment 16•4 years ago
|
||
This is mostly straight-forward cleanup, but there's a behavior change which
was an oversight from bug 1386840, the regular mObservers stuff didn't pass the
right thing (was passing only mWasAlternate rather than whether it was
deferred).
I think that as a result, only in XML documents (since nsXMLContentSink is the
only thing that adds itself as a global CSS loader observer that ever looks at
this boolean), we may end up breaking this assert:
(If there are any sheets with non-matching media queries and such).
But I couldn't find a test-case that broke it (tried SVG / XHTML), and in other
documents like pure XML you cannot specify a media query...
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 17•4 years ago
|
||
Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/cd79f0fbb18e Cleanup a bit PreloadedStyleSheet construction. r=heycam https://hg.mozilla.org/integration/mozilla-inbound/rev/b3618e35bce0 Don't uselessly check for OOM in Loader::PostLoadEvent. r=heycam
Comment 18•4 years ago
|
||
bugherder |
Assignee | ||
Comment 19•4 years ago
|
||
Assignee | ||
Comment 20•4 years ago
|
||
Assignee | ||
Comment 21•4 years ago
|
||
Comment 22•4 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1fbe01c0e461 Move Loader::Sheets out of line. r=heycam
Updated•4 years ago
|
Comment 23•4 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/76b29d6f9e86 Cleanup initialization and usage of Loader::mSyncCallback. r=heycam
Updated•4 years ago
|
Updated•4 years ago
|
Comment 24•4 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2e0d1725d65e Move SheetLoadData out of line too. r=heycam https://hg.mozilla.org/integration/autoland/rev/bec4c05b5a70 Make the parsing mode part of the cache key. r=heycam
Comment 25•4 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4727bbeb8b85 Trivially cleanup a condition. r=heycam
Updated•4 years ago
|
Updated•4 years ago
|
Comment 26•4 years ago
|
||
bugherder |
Comment 27•4 years ago
|
||
Thanks for this cleanup!
Comment 28•4 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4918a9dc15d7 Factor out the stylesheet cache bits. r=heycam https://hg.mozilla.org/integration/autoland/rev/eec457de4550 Cleanup CreateSheet so that it returns its arguments rather than having outparams. r=heycam https://hg.mozilla.org/integration/autoland/rev/cf17f0433abf Move inline sheet creation somewhere else that isn't CreateSheet. r=heycam https://hg.mozilla.org/integration/autoland/rev/db8dfad2e3b3 Make Loader APIs return a Result. r=heycam https://hg.mozilla.org/integration/autoland/rev/589addc490dc Remove various bool arguments in sheet loader APIs. r=heycam https://hg.mozilla.org/integration/autoland/rev/1f4c64e73cfc Don't use nsBaseHashtable in the CSS loader. r=heycam https://hg.mozilla.org/integration/autoland/rev/aec66f88bfd0 Cleanup confusing lifetime of SheetLoadData. r=heycam https://hg.mozilla.org/integration/autoland/rev/a28efee19e9c Cleanup slightly SheetComplete, and use the right boolean to notify. r=heycam https://hg.mozilla.org/integration/autoland/rev/3d9de500e9c5 Make PrepareSheet take a reference. r=heycam https://hg.mozilla.org/integration/autoland/rev/e10941ec60df Move a bit of code around in Loader::LoadInlineSheet. r=heycam https://hg.mozilla.org/integration/autoland/rev/852c5aaa34f7 Remove sheets from the <link> cache when they have been touched by CSS OM. r=heycam
Comment 29•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4918a9dc15d7
https://hg.mozilla.org/mozilla-central/rev/eec457de4550
https://hg.mozilla.org/mozilla-central/rev/cf17f0433abf
https://hg.mozilla.org/mozilla-central/rev/db8dfad2e3b3
https://hg.mozilla.org/mozilla-central/rev/589addc490dc
https://hg.mozilla.org/mozilla-central/rev/1f4c64e73cfc
https://hg.mozilla.org/mozilla-central/rev/aec66f88bfd0
https://hg.mozilla.org/mozilla-central/rev/a28efee19e9c
https://hg.mozilla.org/mozilla-central/rev/3d9de500e9c5
https://hg.mozilla.org/mozilla-central/rev/e10941ec60df
https://hg.mozilla.org/mozilla-central/rev/852c5aaa34f7
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Description
•