From 9fc5a47ce636e5daa425911d8c37ec2bf1789f2e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 20 Nov 2010 23:53:34 +0000 Subject: [PATCH] Don't keep entries for XRC resources that failed to load in wxXmlResource. Attempting to load a resource that couldn't be loaded resulted in wxXmlResource::Load() returning false for this and _all_the_subsequent_ calls to it because each call to Load() reattempted to reload all resources, including the one(s) that failed to load initially. Instead, try to load just the resource(s) that we should load right now and ignore all the other ones. Also, don't add entries for the one(s) that we fail to load. This fixes the unit test failures in the XRC test case which was affected by the test checking that XRC couldn't be loaded from garbage that ran before it. It also makes the code simpler by ensuring that wxXmlResourceDataRecords elements always have a valid wxXmlDocument associated with them. Also clean up the code: use wxScopedPtr instead of manually deleting pointers and reorganize #if checks to be easier to follow. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@66219 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/xrc/xmlres.h | 4 + src/xrc/xmlres.cpp | 265 +++++++++++++++++++++++----------------- 2 files changed, 154 insertions(+), 115 deletions(-) diff --git a/include/wx/xrc/xmlres.h b/include/wx/xrc/xmlres.h index 1530fab993..f7c6867929 100644 --- a/include/wx/xrc/xmlres.h +++ b/include/wx/xrc/xmlres.h @@ -299,6 +299,10 @@ protected: virtual void DoReportError(const wxString& xrcFile, const wxXmlNode *position, const wxString& message); + // Load the contents of a single file and returns its contents as a new + // wxXmlDocument (which will be owned by caller) on success or NULL. + wxXmlDocument *DoLoadFile(const wxString& file); + // Scans the resources list for unloaded files and loads them. Also reloads // files that have been modified since last loading. bool UpdateResources(); diff --git a/src/xrc/xmlres.cpp b/src/xrc/xmlres.cpp index eebc913691..1b62dbc3dc 100644 --- a/src/xrc/xmlres.cpp +++ b/src/xrc/xmlres.cpp @@ -48,16 +48,45 @@ #include "wx/dir.h" #include "wx/xml/xml.h" #include "wx/hashset.h" +#include "wx/scopedptr.h" +namespace +{ + +// Helper function to get modification time of either a wxFileSystem URI or +// just a normal file name, depending on the build. +#if wxUSE_DATETIME + +wxDateTime GetXRCFileModTime(const wxString& filename) +{ +#if wxUSE_FILESYSTEM + wxFileSystem fsys; + wxScopedPtr file(fsys.OpenFile(filename)); + + return file ? file->GetModificationTime() : wxDateTime(); +#else // wxUSE_FILESYSTEM + return wxDateTime(wxFileModificationTime(filename)); +#endif // wxUSE_FILESYSTEM +} + +#endif // wxUSE_DATETIME + +} // anonymous namespace class wxXmlResourceDataRecord { public: - wxXmlResourceDataRecord() : Doc(NULL) { + // Ctor takes ownership of the document pointer. + wxXmlResourceDataRecord(const wxString& File_, + wxXmlDocument *Doc_ + ) + : File(File_), Doc(Doc_) + { #if wxUSE_DATETIME - Time = wxDateTime::Now(); + Time = GetXRCFileModTime(File); #endif } + ~wxXmlResourceDataRecord() {delete Doc;} wxString File; @@ -65,6 +94,8 @@ public: #if wxUSE_DATETIME wxDateTime Time; #endif + + wxDECLARE_NO_COPY_CLASS(wxXmlResourceDataRecord); }; class wxXmlResourceDataRecords : public wxVector @@ -314,6 +345,8 @@ bool wxXmlResource::Load(const wxString& filemask_) { wxString filemask = ConvertFileNameToURL(filemask_); + bool allOK = true; + #if wxUSE_FILESYSTEM wxFileSystem fsys; # define wxXmlFindFirst fsys.FindFirst(filemask, wxFILE) @@ -335,14 +368,16 @@ bool wxXmlResource::Load(const wxString& filemask_) if ( IsArchive(fnd) ) { if ( !Load(fnd + wxT("#zip:*.xrc")) ) - return false; + allOK = false; } else // a single resource URL #endif // wxUSE_FILESYSTEM { - wxXmlResourceDataRecord *drec = new wxXmlResourceDataRecord; - drec->File = fnd; - Data().push_back(drec); + wxXmlDocument * const doc = DoLoadFile(fnd); + if ( !doc ) + allOK = false; + else + Data().push_back(new wxXmlResourceDataRecord(fnd, doc)); } fnd = wxXmlFindNext; @@ -350,7 +385,7 @@ bool wxXmlResource::Load(const wxString& filemask_) # undef wxXmlFindFirst # undef wxXmlFindNext - return UpdateResources(); + return allOK; } bool wxXmlResource::Unload(const wxString& filename) @@ -613,133 +648,133 @@ static void PreprocessForIdRanges(wxXmlNode *rootnode) bool wxXmlResource::UpdateResources() { bool rt = true; - bool modif; -# if wxUSE_FILESYSTEM - wxFSFile *file = NULL; - wxUnusedVar(file); - wxFileSystem fsys; -# endif - - wxString encoding(wxT("UTF-8")); -#if !wxUSE_UNICODE && wxUSE_INTL - if ( (GetFlags() & wxXRC_USE_LOCALE) == 0 ) - { - // In case we are not using wxLocale to translate strings, convert the - // strings GUI's charset. This must not be done when wxXRC_USE_LOCALE - // is on, because it could break wxGetTranslation lookup. - encoding = wxLocale::GetSystemEncodingName(); - } -#endif for ( wxXmlResourceDataRecords::iterator i = Data().begin(); i != Data().end(); ++i ) { wxXmlResourceDataRecord* const rec = *i; - modif = (rec->Doc == NULL); + // Check if we need to reload this one. - if (!modif && !(m_flags & wxXRC_NO_RELOADING)) + // We never do it if this flag is specified. + if ( m_flags & wxXRC_NO_RELOADING ) + continue; + + // Otherwise check its modification time if we can. +#if wxUSE_DATETIME + const wxDateTime lastModTime = GetXRCFileModTime(rec->File); + + if ( lastModTime.IsValid() && lastModTime <= rec->Time ) +#else // !wxUSE_DATETIME + // Never reload the file contents: we can't know whether it changed or + // not in this build configuration and it would be unexpected and + // counter-productive to get a performance hit (due to constant + // reloading of XRC files) in a minimal wx build which is presumably + // used because of resource constraints of the current platform. +#endif // wxUSE_DATETIME/!wxUSE_DATETIME { -# if wxUSE_FILESYSTEM - file = fsys.OpenFile(rec->File); -# if wxUSE_DATETIME - modif = file && file->GetModificationTime() > rec->Time; -# else // wxUSE_DATETIME - modif = true; -# endif // wxUSE_DATETIME - if (!file) - { - wxLogError(_("Cannot open file '%s'."), rec->File); - rt = false; - } - wxDELETE(file); - wxUnusedVar(file); -# else // wxUSE_FILESYSTEM -# if wxUSE_DATETIME - modif = wxDateTime(wxFileModificationTime(rec->File)) > rec->Time; -# else // wxUSE_DATETIME - modif = true; -# endif // wxUSE_DATETIME -# endif // wxUSE_FILESYSTEM + // No need to reload, the file wasn't modified since we did it + // last. + continue; } - if (modif) + wxXmlDocument * const doc = DoLoadFile(rec->File); + if ( !doc ) { - wxLogTrace(wxT("xrc"), wxT("opening file '%s'"), rec->File); + // Notice that we keep the old XML document: it seems better to + // preserve it instead of throwing it away if we have nothing to + // replace it with. + rt = false; + continue; + } - wxInputStream *stream = NULL; + // Replace the old resource contents with the new one. + delete rec->Doc; + rec->Doc = doc; -# if wxUSE_FILESYSTEM - file = fsys.OpenFile(rec->File); - if (file) - stream = file->GetStream(); -# else - stream = new wxFileInputStream(rec->File); -# endif + // And, now that we loaded it successfully, update the last load time. +#if wxUSE_DATETIME + rec->Time = lastModTime.IsValid() ? lastModTime : wxDateTime::Now(); +#endif // wxUSE_DATETIME + } - if (stream) - { - delete rec->Doc; - rec->Doc = new wxXmlDocument; - } - if (!stream || !stream->IsOk() || !rec->Doc->Load(*stream, encoding)) - { - wxLogError(_("Cannot load resources from file '%s'."), - rec->File); - wxDELETE(rec->Doc); - rt = false; - } - else if (rec->Doc->GetRoot()->GetName() != wxT("resource")) - { - ReportError - ( - rec->Doc->GetRoot(), - "invalid XRC resource, doesn't have root node " - ); - wxDELETE(rec->Doc); - rt = false; - } - else - { - long version; - int v1, v2, v3, v4; - wxString verstr = rec->Doc->GetRoot()->GetAttribute( - wxT("version"), wxT("0.0.0.0")); - if (wxSscanf(verstr.c_str(), wxT("%i.%i.%i.%i"), - &v1, &v2, &v3, &v4) == 4) - version = v1*256*256*256+v2*256*256+v3*256+v4; - else - version = 0; - if (m_version == -1) - m_version = version; - if (m_version != version) - { - wxLogError("Resource files must have same version number."); - rt = false; - } + return rt; +} + +wxXmlDocument *wxXmlResource::DoLoadFile(const wxString& filename) +{ + wxLogTrace(wxT("xrc"), wxT("opening file '%s'"), filename); + + wxInputStream *stream = NULL; - ProcessPlatformProperty(rec->Doc->GetRoot()); - PreprocessForIdRanges(rec->Doc->GetRoot()); - wxIdRangeManager::Get()->FinaliseRanges(rec->Doc->GetRoot()); -#if wxUSE_DATETIME #if wxUSE_FILESYSTEM - rec->Time = file->GetModificationTime(); -#else // wxUSE_FILESYSTEM - rec->Time = wxDateTime(wxFileModificationTime(rec->File)); -#endif // wxUSE_FILESYSTEM -#endif // wxUSE_DATETIME - } + wxFileSystem fsys; + wxScopedPtr file(fsys.OpenFile(filename)); + if (file) + { + // Notice that we don't have ownership of the stream in this case, it + // remains owned by wxFSFile. + stream = file->GetStream(); + } +#else // !wxUSE_FILESYSTEM + wxFileInputStream fstream(filename); + stream = &fstream; +#endif // wxUSE_FILESYSTEM/!wxUSE_FILESYSTEM -# if wxUSE_FILESYSTEM - wxDELETE(file); - wxUnusedVar(file); -# else - wxDELETE(stream); -# endif - } + if ( !stream || !stream->IsOk() ) + { + wxLogError(_("Cannot open resources file '%s'."), filename); + return NULL; } - return rt; + wxString encoding(wxT("UTF-8")); +#if !wxUSE_UNICODE && wxUSE_INTL + if ( (GetFlags() & wxXRC_USE_LOCALE) == 0 ) + { + // In case we are not using wxLocale to translate strings, convert the + // strings GUI's charset. This must not be done when wxXRC_USE_LOCALE + // is on, because it could break wxGetTranslation lookup. + encoding = wxLocale::GetSystemEncodingName(); + } +#endif + + wxScopedPtr doc(new wxXmlDocument); + if (!doc->Load(*stream, encoding)) + { + wxLogError(_("Cannot load resources from file '%s'."), filename); + return NULL; + } + + wxXmlNode * const root = doc->GetRoot(); + if (root->GetName() != wxT("resource")) + { + ReportError + ( + root, + "invalid XRC resource, doesn't have root node " + ); + return NULL; + } + + long version; + int v1, v2, v3, v4; + wxString verstr = root->GetAttribute(wxT("version"), wxT("0.0.0.0")); + if (wxSscanf(verstr, wxT("%i.%i.%i.%i"), &v1, &v2, &v3, &v4) == 4) + version = v1*256*256*256+v2*256*256+v3*256+v4; + else + version = 0; + if (m_version == -1) + m_version = version; + if (m_version != version) + { + wxLogWarning("Resource files must have same version number."); + } + + ProcessPlatformProperty(root); + PreprocessForIdRanges(root); + wxIdRangeManager::Get()->FinaliseRanges(root); + + return doc.release(); } wxXmlNode *wxXmlResource::DoFindResource(wxXmlNode *parent, -- 2.47.2