Changeset 62937 in webkit

Timestamp:

Jul 9, 2010 4:33:17 AM (14 years ago)

Author:

commit-queue@webkit.org

Message:

2010-07-09 Andreas Wictor <​andreas.wictor@xcerion.com>

Reviewed by Alexey Proskuryakov.

Remove global variables from XSLTProcessorLibxslt.cpp
​https://bugs.webkit.org/show_bug.cgi?id=41348

Remove the globalProcessor and globalDocLoader global variables
by using the _private field that exists on most libxml structs.

No new tests, existing tests covers this.

• xml/XSLTProcessor.h: (WebCore::XSLTProcessor::sourceNode): (WebCore::XSLTProcessor::XSLTProcessor):
• xml/XSLTProcessorLibxslt.cpp: (WebCore::registeredXSLTProcessors): (WebCore::registeredXSLStyleSheets): (WebCore::docLoaderFunc): (WebCore::clearSavedStyleSheetPointers): (WebCore::xsltStylesheetPointer): (WebCore::XSLTProcessor::transformToString):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• xml/XSLTProcessor.h (modified) (2 diffs)
• xml/XSLTProcessorLibxslt.cpp (modified) (12 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-07-09  Andreas Wictor  <andreas.wictor@xcerion.com>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Remove global variables from XSLTProcessorLibxslt.cpp
+        https://bugs.webkit.org/show_bug.cgi?id=41348
+
+        Remove the globalProcessor and globalDocLoader global variables
+        by using the _private field that exists on most libxml structs.
+
+        No new tests, existing tests covers this.
+
+        * xml/XSLTProcessor.h:
+        (WebCore::XSLTProcessor::sourceNode):
+        (WebCore::XSLTProcessor::XSLTProcessor):
+        * xml/XSLTProcessorLibxslt.cpp:
+        (WebCore::registeredXSLTProcessors):
+        (WebCore::registeredXSLStyleSheets):
+        (WebCore::docLoaderFunc):
+        (WebCore::clearSavedStyleSheetPointers):
+        (WebCore::xsltStylesheetPointer):
+        (WebCore::XSLTProcessor::transformToString):
+
 2010-07-09  Adam Barth  <abarth@webkit.org>
 

trunk/WebCore/xml/XSLTProcessor.h

@@ -68 +68 @@
     // Only for libXSLT callbacks
     XSLStyleSheet* xslStylesheet() const { return m_stylesheet.get(); }
+    Node* sourceNode() const { return m_sourceNode; }
 #endif
 
@@ -73 +74 @@
 
 private:
-    XSLTProcessor() { }
+    XSLTProcessor() : m_sourceNode(0) { }
 
     RefPtr<XSLStyleSheet> m_stylesheet;
     RefPtr<Node> m_stylesheetRootNode;
     ParameterMap m_parameters;
+
+    Node* m_sourceNode; // Source node is only non-null in transformToString(), so this cannot become a dangling pointer.
 };
 

trunk/WebCore/xml/XSLTProcessorLibxslt.cpp

@@ -46 +46 @@
 #include <libxslt/xsltutils.h>
 #include <wtf/Assertions.h>
+#include <wtf/HashSet.h>
 #include <wtf/text/CString.h>
 #include <wtf/Vector.h>
@@ -95 +96 @@
 }
 
-// FIXME: There seems to be no way to control the ctxt pointer for loading here, thus we have globals.
-static XSLTProcessor* globalProcessor = 0;
-static DocLoader* globalDocLoader = 0;
+static HashSet<XSLTProcessor*>& registeredXSLTProcessors()
+{
+    DEFINE_STATIC_LOCAL(HashSet<XSLTProcessor*>, xsltProcessors, ());
+    return xsltProcessors;
+}
+
+static HashSet<XSLStyleSheet*>& registeredXSLStyleSheets()
+{
+    DEFINE_STATIC_LOCAL(HashSet<XSLStyleSheet*>, xslStyleSheets, ());
+    return xslStyleSheets;
+}
+
 static xmlDocPtr docLoaderFunc(const xmlChar* uri,
                                     xmlDictPtr,
@@ -104 +114 @@
                                     xsltLoadType type)
 {
-    if (!globalProcessor)
-        return 0;
-
     switch (type) {
     case XSLT_LOAD_DOCUMENT: {
-        xsltTransformContextPtr context = (xsltTransformContextPtr)ctxt;
+        xsltTransformContextPtr context = static_cast<xsltTransformContextPtr>(ctxt);
+        XSLTProcessor* processor = static_cast<XSLTProcessor*>(context->_private);
+        if (!processor || HashTraits<XSLTProcessor*>::isDeletedValue(processor)
+            || !registeredXSLTProcessors().contains(processor)) {
+            static bool errorMessagePrinted = false;
+            if (!errorMessagePrinted) {
+                fprintf(stderr, "WebKit XSLT document loader was called with unknown transformation context.");
+                errorMessagePrinted = true;
+            }
+            return 0;
+        }
+
+        RefPtr<Document> ownerDocument = processor->sourceNode()->document();
+        DocLoader* docLoader = ownerDocument->docLoader();
+
         xmlChar* base = xmlNodeGetBase(context->document->doc, context->node);
         KURL url(KURL(ParsedURLString, reinterpret_cast<const char*>(base)), reinterpret_cast<const char*>(uri));
@@ -118 +139 @@
         Vector<char> data;
 
-        bool requestAllowed = globalDocLoader->frame() && globalDocLoader->doc()->securityOrigin()->canRequest(url);
+        bool requestAllowed = docLoader->frame() && docLoader->doc()->securityOrigin()->canRequest(url);
         if (requestAllowed) {
-            globalDocLoader->frame()->loader()->loadResourceSynchronously(url, AllowStoredCredentials, error, response, data);
-            requestAllowed = globalDocLoader->doc()->securityOrigin()->canRequest(response.url());
+            docLoader->frame()->loader()->loadResourceSynchronously(url, AllowStoredCredentials, error, response, data);
+            requestAllowed = docLoader->doc()->securityOrigin()->canRequest(response.url());
         }
         if (!requestAllowed) {
             data.clear();
-            globalDocLoader->printAccessDeniedMessage(url);
+            docLoader->printAccessDeniedMessage(url);
         }
 
         Console* console = 0;
-        if (Frame* frame = globalProcessor->xslStylesheet()->ownerDocument()->frame())
+        if (Frame* frame = processor->xslStylesheet()->ownerDocument()->frame())
             console = frame->domWindow()->console();
         xmlSetStructuredErrorFunc(console, XSLTProcessor::parseErrorFunc);
@@ -143 +164 @@
         return doc;
     }
-    case XSLT_LOAD_STYLESHEET:
-        return globalProcessor->xslStylesheet()->locateStylesheetSubResource(((xsltStylesheetPtr)ctxt)->doc, uri);
+    case XSLT_LOAD_STYLESHEET: {
+        xsltStylesheetPtr style = static_cast<xsltStylesheetPtr>(ctxt);
+        XSLStyleSheet* xslStyleSheet = static_cast<XSLStyleSheet*>(style->doc->_private);
+        if (!xslStyleSheet || HashTraits<XSLStyleSheet*>::isDeletedValue(xslStyleSheet)
+            || !registeredXSLStyleSheets().contains(xslStyleSheet)) {
+            static bool errorMessagePrinted = false;
+            if (!errorMessagePrinted) {
+                fprintf(stderr, "WebKit XSLT document loader was called with unknown transformation context.");
+                errorMessagePrinted = true;
+            }
+            return 0;
+        }
+
+        xmlDocPtr result = xslStyleSheet->locateStylesheetSubResource(style->doc, uri);
+        if (!result)
+            return 0;
+
+        // Save a pointer to the root stylesheet so that we can access it again from
+        // this callback function if this xsl document imports another xsl document.
+        ASSERT(!result->_private);
+        result->_private = xslStyleSheet;
+        return result;
+    }
     default:
         break;
@@ -150 +192 @@
 
     return 0;
-}
-
-static inline void setXSLTLoadCallBack(xsltDocLoaderFunc func, XSLTProcessor* processor, DocLoader* loader)
-{
-    xsltSetLoaderFunc(func);
-    globalProcessor = processor;
-    globalDocLoader = loader;
 }
 
@@ -222 +257 @@
 }
 
+static void clearSavedStyleSheetPointers(xsltStylesheetPtr style)
+{
+    if (!style || !style->doc)
+        return;
+
+    ASSERT(style->doc->_private);
+    style->doc->_private = 0;
+
+    for (xsltStylesheetPtr import = style->imports; import; import = import->next)
+        clearSavedStyleSheetPointers(import);
+}
+
 static xsltStylesheetPtr xsltStylesheetPointer(RefPtr<XSLStyleSheet>& cachedStylesheet, Node* stylesheetRootNode)
 {
@@ -231 +278 @@
     }
 
-    if (!cachedStylesheet || !cachedStylesheet->document())
+    if (!cachedStylesheet)
         return 0;
 
-    return cachedStylesheet->compileStyleSheet();
+    xmlDocPtr xslDocument = cachedStylesheet->document();
+    if (!xslDocument)
+        return 0;
+
+    // Save a pointer to the stylesheet so that we can access it from the libxslt loader callback.
+    void* old = xslDocument->_private;
+    xslDocument->_private = cachedStylesheet.get();
+    registeredXSLStyleSheets().add(cachedStylesheet.get());
+
+    xsltStylesheetPtr result = cachedStylesheet->compileStyleSheet();
+    registeredXSLStyleSheets().remove(cachedStylesheet.get());
+    clearSavedStyleSheetPointers(result);
+    xslDocument->_private = old;
+    return result;
 }
 
@@ -274 +334 @@
 bool XSLTProcessor::transformToString(Node* sourceNode, String& mimeType, String& resultString, String& resultEncoding)
 {
-    RefPtr<Document> ownerDocument = sourceNode->document();
-
-    setXSLTLoadCallBack(docLoaderFunc, this, ownerDocument->docLoader());
+    xsltSetLoaderFunc(docLoaderFunc);
     xsltStylesheetPtr sheet = xsltStylesheetPointer(m_stylesheet, m_stylesheetRootNode.get());
     if (!sheet) {
-        setXSLTLoadCallBack(0, 0, 0);
+        xsltSetLoaderFunc(0);
         return false;
     }
@@ -298 +356 @@
         registerXSLTExtensions(transformContext);
 
+        // Save a pointer to this XSLTProcessor so that we can access it from the libxslt loader callback.
+        ASSERT(!transformContext->_private);
+        transformContext->_private = this;
+        registeredXSLTProcessors().add(this);
+        m_sourceNode = sourceNode;
+
         // <http://bugs.webkit.org/show_bug.cgi?id=16077>: XSLT processor <xsl:sort> algorithm only compares by code point.
         xsltSetCtxtSortFunc(transformContext, xsltUnicodeSortFunction);
@@ -310 +374 @@
         xmlDocPtr resultDoc = xsltApplyStylesheetUser(sheet, sourceDoc, 0, 0, 0, transformContext);
 
+        registeredXSLTProcessors().remove(this);
         xsltFreeTransformContext(transformContext);
         freeXsltParamArray(params);
@@ -324 +389 @@
 
     sheet->method = origMethod;
-    setXSLTLoadCallBack(0, 0, 0);
+    xsltSetLoaderFunc(0);
     xsltFreeStylesheet(sheet);
     m_stylesheet = 0;
+    m_sourceNode = 0;
 
     return success;