As far as I can tell these are only used in one[1] test in mozilla central. [1] https://searchfox.org/mozilla-central/rev/b80994a43e5d92c2f79160ece176127eed85dcc9/chrome/test/unit/data/test_bug564667/chrome.manifest#16

Thanks for the heads-up, Richard will handle it.

Ideally I'd like to land bug 1448162 first, as some of this patch built on top of that.

Comment on attachment 8964448 [details] Bug 1450753 - Remove XUL style overlays. https://reviewboard.mozilla.org/r/233178/#review239008 r- only because I'm worried about the nsISerializable implementation changes. ::: dom/xul/nsXULPrototypeDocument.cpp (Diff revision 1) > - uint32_t count, i; > - nsCOMPtr<nsIURI> styleOverlayURI; > - > - rv = aStream->Read32(&count); > - if (NS_FAILED(rv)) { > - return rv; > - } > - > - for (i = 0; i < count; ++i) { > - rv = aStream->ReadObject(true, getter_AddRefs(supports)); > - if (NS_FAILED(rv)) { > - return rv; > - } > - styleOverlayURI = do_QueryInterface(supports); > - mStyleSheetReferences.AppendObject(styleOverlayURI); > - } > - > - I'm mildly worried about this particular change, as we occasionally persist nsISerializable objects to disk, and read them from disk. I don't know if these objects are ever written to disk, but I think I would be more comfortable if we continue reading/writing the count uint32_t, and report an error if we read a non-zero value. I'd also like to add a comment there mentioning why the value is there for legacy purposes. ::: dom/xul/nsXULPrototypeDocument.cpp (Diff revision 1) > - uint32_t count; > - > - count = mStyleSheetReferences.Count(); > - nsresult tmp = aStream->Write32(count); > - if (NS_FAILED(tmp)) { > - rv = tmp; > - } > - > - uint32_t i; > - for (i = 0; i < count; ++i) { > - tmp = aStream->WriteCompoundObject(mStyleSheetReferences[i], > - NS_GET_IID(nsIURI), true); > - if (NS_FAILED(tmp)) { > - rv = tmp; > - } > - } (this change too for the same reasons as above)

Comment on attachment 8964448 [details] Bug 1450753 - Remove XUL style overlays. https://reviewboard.mozilla.org/r/233178/#review239008 > I'm mildly worried about this particular change, as we occasionally persist nsISerializable objects to disk, and read them from disk. > > I don't know if these objects are ever written to disk, but I think I would be more comfortable if we continue reading/writing the count uint32_t, and report an error if we read a non-zero value. > > I'd also like to add a comment there mentioning why the value is there for legacy purposes. I may be missing something as this is my first run in with this caching code, but it look like these are only every written to the startup cache. If I follow everything it looks like we wipe out this cache on any update/version change: Starting over at https://searchfox.org/mozilla-central/rev/a0665934fa05158a5a943d4c8b277465910c029c/toolkit/xre/nsAppRunner.cpp#4304 - first we check version - we should get either cachesOK=false or versionOK=false which then sets startupCacheValid=false - calls StartupCache::IgnoreDiskCache(). That either directly wipes the cache or later deletes it in StartupCache::Init().

(In reply to Brendan Dahl [:bdahl] from comment #5) > I may be missing something as this is my first run in with this caching > code, but it look like these are only every written to the startup cache. If > I follow everything it looks like we wipe out this cache on any > update/version change: > > > Starting over at > https://searchfox.org/mozilla-central/rev/ > a0665934fa05158a5a943d4c8b277465910c029c/toolkit/xre/nsAppRunner.cpp#4304 > - first we check version > - we should get either cachesOK=false or versionOK=false which then sets > startupCacheValid=false > - calls StartupCache::IgnoreDiskCache(). That either directly wipes the > cache or later deletes it in StartupCache::Init(). Had a conversation with olli on IRC about this (https://mozilla.logbot.info/content/20180404#c14559961). I want to read through the relevant code to convince myself that this is OK before we land it, which I probably won't get around to today. Opening a ni? on myself here so I don't forget to do it.

Comment on attachment 8964448 [details] Bug 1450753 - Remove XUL style overlays. https://reviewboard.mozilla.org/r/233178/#review254490 I've convinced myself this is OK. Thanks :-)

Pushed by bdahl@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0673137d307f Remove XUL style overlays. r=Nika