Changeset 46418 in webkit

Timestamp:

Jul 27, 2009 10:04:35 AM (15 years ago)

Author:

Nikolas Zimmermann

Message:

[WML] History handling / page cache / loading is buggy and depends on several hacks
​https://bugs.webkit.org/show_bug.cgi?id=27707

Redesign WML history/loading handling. See ChangeLog for details.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/wml/enter-card-with-events-expected.txt (added)
• LayoutTests/wml/enter-card-with-events.html (added)
• LayoutTests/wml/enter-first-card-with-events-expected.txt (added)
• LayoutTests/wml/enter-first-card-with-events.html (added)
• LayoutTests/wml/resources/WMLTestCase.js (modified) (3 diffs)
• LayoutTests/wml/resources/enter-card-with-events.js (added)
• LayoutTests/wml/resources/enter-card-with-events.wml (added)
• LayoutTests/wml/resources/enter-first-card-with-events.js (added)
• LayoutTests/wml/resources/enter-first-card-with-events.wml (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/dom/Document.cpp (modified) (1 diff)
• WebCore/dom/Document.h (modified) (2 diffs)
• WebCore/history/BackForwardList.cpp (modified) (1 diff)
• WebCore/history/BackForwardList.h (modified) (1 diff)
• WebCore/loader/FrameLoader.cpp (modified) (5 diffs)
• WebCore/loader/FrameLoader.h (modified) (3 diffs)
• WebCore/wml/WMLCardElement.cpp (modified) (3 diffs)
• WebCore/wml/WMLGoElement.cpp (modified) (2 diffs)
• WebCore/wml/WMLPageState.cpp (modified) (4 diffs)
• WebCore/wml/WMLPageState.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2009-07-27  Nikolas Zimmermann  <nikolas.zimmermann@torchmobile.com>
+
+        Reviewed by George Staikos.
+
+        [WML] History handling / page cache / loading is buggy and depends on several hacks
+        https://bugs.webkit.org/show_bug.cgi?id=27707
+
+        Add two new WML layout tests after redesigning WML history/loading handling.
+
+        * wml/enter-card-with-events-expected.txt: Added.
+        * wml/enter-card-with-events.html: Added.
+        * wml/enter-first-card-with-events-expected.txt: Added.
+        * wml/enter-first-card-with-events.html: Added.
+        * wml/resources/WMLTestCase.js:
+        * wml/resources/enter-card-with-events.js: Added.
+        (setupTestDocument):
+        (prepareTest):
+        (executeTest):
+        * wml/resources/enter-card-with-events.wml: Added.
+        * wml/resources/enter-first-card-with-events.js: Added.
+        (setupTestDocument):
+        (prepareTest):
+        (executeTest):
+        * wml/resources/enter-first-card-with-events.wml: Added.
+
 2009-07-27  Jan Michael Alonzo  <jmalonzo@webkit.org>
 

trunk/LayoutTests/wml/resources/WMLTestCase.js

@@ -10 +10 @@
 }
 
-function createWMLTestCase(testDescription, substitutesVariables, testName, executeImmediately) {
+function createWMLTestCase(testDescription, substitutesVariables, testName, executeImmediately, needsReset) {
     // Setup default test options
     if (substitutesVariables == null) {
@@ -24 +24 @@
     }
 
+    // Some tests may want to handle resetting the page state themselves
+    if (needsReset == null) {
+        needsReset = true;
+    }
+
     // Initialize JS test
     description(testDescription);
@@ -29 +34 @@
 
     // Clear variable state & history
-    document.resetWMLPageState();
+    if (needsReset)
+        document.resetWMLPageState();
 
     // Setup DRT specific settings

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-07-27  Nikolas Zimmermann  <nikolas.zimmermann@torchmobile.com>
+
+        Reviewed by George Staikos.
+
+        [WML] History handling / page cache / loading is buggy and depends on several hacks
+        https://bugs.webkit.org/show_bug.cgi?id=27707
+
+        Redesign WML history/loading handling. In detail:
+
+        - Remove FrameLoader::setForceReloadWmlDeck(). WML used to force a special loading behaviour
+          by calling this method from WMLGoElement & friends - instead teach FrameLoader to detect
+          WML content itself.
+
+          WML content is usually a standalone WML document (isWMLDocument()=true) or as special case
+          an XHTML document which embeds a WML document (that's the way the WML layout tests work).
+          Force WML loading behaviour even for XHTML document which embed WML documents. This only
+          applies to our layout tests, not for any real world site. Though it gives us a perfect
+          way to test the WML loading code even when we're not operating on a standalone WML document.
+
+          Whenever a WMLCardElement is inserted into the document it will check wheter it's inserted
+          in a standalone WML document or wheter the main frame document is different. If it differs
+          the main frame documents' "containsWMLContent" property is set to true.
+
+          -> Make FrameLoader::shouldReload() use the new frameContainsWMLContent() method, which
+             checks if the associated frame document is a WML document or wheter it contains WML content.
+
+        - Change FrameLoader::loadItem() to use the new frameContainsWMLContent() method for 'shouldScroll'
+          detection. WML documents (or those containing WML content) always want new loads even for in-page
+          navigation. No "scroll to anchor" mechanism should apply.
+
+        - Modify FrameLoader::canCachePageContainingThisFrame() to check for !frameContainsWMLContent().
+          WML pages should never be cached, potential security problem due the use of variables (per spec).
+
+        Add two new WML tests which were broken before, testing onenterforward/onenterbackward event handling
+        and history navigation (<prev/> task).
+
+        Tests: wml/enter-card-with-events.html
+               wml/enter-first-card-with-events.html
+
+        * dom/Document.cpp: Initialize new 'm_containsWMLContent' property.
+        (WebCore::Document::Document):
+        * dom/Document.h: Add new helper methods and 'm_containsWMLContent" variable (explained above).
+        (WebCore::Document::setContainsWMLContent):
+        (WebCore::Document::containsWMLContent):
+        * history/BackForwardList.cpp:
+        (WebCore::BackForwardList::clearWMLPageHistory): Renamed from clearWmlPageHistory() & slight cleanup.
+        * history/BackForwardList.h:
+        * loader/FrameLoader.cpp: Rework WML loading behaviour (explained above).
+        (WebCore::FrameLoader::FrameLoader):
+        (WebCore::frameContainsWMLContent):
+        (WebCore::FrameLoader::canCachePageContainingThisFrame):
+        (WebCore::FrameLoader::shouldReload):
+        (WebCore::FrameLoader::loadItem):
+        * loader/FrameLoader.h:
+        * wml/WMLCardElement.cpp: 
+        (WebCore::WMLCardElement::handleIntrinsicEventIfNeeded): No need anymore to manually track history length.
+        (WebCore::WMLCardElement::insertedIntoDocument): Handle setting containsWMLContent on the main frame document.
+        * wml/WMLGoElement.cpp: 
+        (WebCore::WMLGoElement::executeTask): Remove call to FrameLoader::setForceReloadWmlDeck()
+        * wml/WMLPageState.cpp: Remove 'm_historyLength' - no need anymore to track history length on our own.
+        (WebCore::WMLPageState::WMLPageState):
+        (WebCore::WMLPageState::dump):
+        (WebCore::WMLPageState::reset):
+        * wml/WMLPageState.h:
+
 2009-07-27  Pavel Feldman  <pfeldman@chromium.org>
 

trunk/WebCore/dom/Document.cpp

@@ -336 +336 @@
 #endif
     , m_usingGeolocation(false)
+#if ENABLE(WML)
+    , m_containsWMLContent(false)
+#endif
 {
     m_document.resetSkippingRef(this);

trunk/WebCore/dom/Document.h

@@ -1035 +1035 @@
 
 #if ENABLE(WML)
+    void setContainsWMLContent(bool value) { m_containsWMLContent = value; }
+    bool containsWMLContent() const { return m_containsWMLContent; }
+
     void resetWMLPageState();
     void initializeWMLPageState();
@@ -1117 +1120 @@
     
     bool m_usingGeolocation;
+
+#if ENABLE(WML)
+    bool m_containsWMLContent;
+#endif
 };
 

trunk/WebCore/history/BackForwardList.cpp

@@ -267 +267 @@
 
 #if ENABLE(WML)
-void BackForwardList::clearWmlPageHistory()
-{
-    PassRefPtr<HistoryItem> cur = currentItem();
-
-    for (unsigned i = 0; i < m_entries.size(); ++i)
+void BackForwardList::clearWMLPageHistory()
+{
+    int size = m_entries.size();
+    for (int i = 0; i < size; ++i)
         pageCache()->remove(m_entries[i].get());
 

trunk/WebCore/history/BackForwardList.h

@@ -98 +98 @@
     
 #if ENABLE(WML)
-    void clearWmlPageHistory();
+    void clearWMLPageHistory();
 #endif
 

trunk/WebCore/loader/FrameLoader.cpp

@@ -272 +272 @@
     , m_didDispatchDidCommitLoad(false)
 #endif
-#if ENABLE(WML)
-    , m_forceReloadWmlDeck(false)
-#endif
 {
 }
@@ -1784 +1781 @@
 }
 
+#if ENABLE(WML)
+static inline bool frameContainsWMLContent(Frame* frame)
+{
+    Document* document = frame ? frame->document() : 0;
+    if (!document)
+        return false;
+
+    return document->containsWMLContent() || document->isWMLDocument();
+}
+#endif
+
 bool FrameLoader::canCachePageContainingThisFrame()
 {
@@ -1811 +1819 @@
         && !m_documentLoader->applicationCache()
         && !m_documentLoader->candidateApplicationCacheGroup()
+#endif
+#if ENABLE(WML)
+        && !frameContainsWMLContent(m_frame)
 #endif
         && m_client->canCachePage()
@@ -2964 +2975 @@
 }
 
+bool FrameLoader::shouldReload(const KURL& currentURL, const KURL& destinationURL)
+{
 #if ENABLE(WML)
-void FrameLoader::setForceReloadWmlDeck(bool reload)
-{
-    m_forceReloadWmlDeck = reload;
-}
-#endif
-
-bool FrameLoader::shouldReload(const KURL& currentURL, const KURL& destinationURL)
-{
-#if ENABLE(WML)
-    // As for WML deck, sometimes it's supposed to be reloaded even if the same URL with fragment
-    if (m_forceReloadWmlDeck)
+    // All WML decks are supposed to be reloaded, even within the same URL fragment
+    if (frameContainsWMLContent(m_frame))
         return true;
 #endif
@@ -4430 +4434 @@
 
 #if ENABLE(WML)
-    if (m_frame->document()->isWMLDocument())
+    // All WML decks should go through the real load mechanism, not the scroll-to-anchor code
+    if (frameContainsWMLContent(m_frame))
         shouldScroll = false;
 #endif

trunk/WebCore/loader/FrameLoader.h

@@ -1 +1 @@
 /*
  * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
- * Copyright (C) 2008 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
+ * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  *
  * Redistribution and use in source and binary forms, with or without
@@ -237 +237 @@
         void didFirstVisuallyNonEmptyLayout();
 
-#if ENABLE(WML)
-        void setForceReloadWmlDeck(bool);
-#endif
-
         void loadedResourceFromMemoryCache(const CachedResource*);
         void tellClientAboutPastMemoryCacheLoads();
@@ -626 +622 @@
         bool m_didDispatchDidCommitLoad;
 #endif
-
-#if ENABLE(WML)
-        bool m_forceReloadWmlDeck;
-#endif
     };
 

trunk/WebCore/wml/WMLCardElement.cpp

@@ -29 +29 @@
 #include "MappedAttribute.h"
 #include "NodeList.h"
+#include "Page.h"
 #include "RenderStyle.h"
 #include "WMLDocument.h"
@@ -132 +133 @@
     if (!loader)
         return;
-    
-    int currentHistoryLength = loader->getHistoryLength();
-    int lastHistoryLength = pageState->historyLength();
 
     // Calculate the entry method of current card 
     WMLIntrinsicEventType eventType = WMLIntrinsicEventUnknown;
-    if (lastHistoryLength > currentHistoryLength)
+
+    switch (loader->loadType()) {
+    case FrameLoadTypeReload:
+        break;
+    case FrameLoadTypeBack:
         eventType = WMLIntrinsicEventOnEnterBackward;
-    else if (lastHistoryLength < currentHistoryLength)
+        break;
+    default:
         eventType = WMLIntrinsicEventOnEnterForward;
-
-    // Synchronize history length with WMLPageState
-    pageState->setHistoryLength(currentHistoryLength);
- 
+        break;
+    }
+
     // Figure out target event handler
     WMLIntrinsicEventHandler* eventHandler = this->eventHandler();
@@ -242 +244 @@
 {
     WMLElement::insertedIntoDocument();
+    Document* document = this->document();
 
     // The first card inserted into a document, is visible by default.
     if (!m_isVisible) {
-        RefPtr<NodeList> nodeList = document()->getElementsByTagName("card");
+        RefPtr<NodeList> nodeList = document->getElementsByTagName("card");
         if (nodeList && nodeList->length() == 1 && nodeList->item(0) == this)
             m_isVisible = true;
+    }
+
+    // For the WML layout tests we embed WML content in a XHTML document. Navigating to different cards
+    // within the same deck has a different behaviour in HTML than in WML. HTML wants to "scroll to anchor"
+    // (see FrameLoader) but WML wants a reload. Notify the root document of the layout test that we want
+    // to mimic WML behaviour. This is rather tricky, but has been tested extensively. Usually it's not possible
+    // at all to embed WML in HTML, it's not designed that way, we're just "abusing" it for dynamically created layout tests.
+    if (document->page() && document->page()->mainFrame()) {
+        Document* rootDocument = document->page()->mainFrame()->document();
+        if (rootDocument && rootDocument != document)
+            rootDocument->setContainsWMLContent(true);
     }
 }

trunk/WebCore/wml/WMLGoElement.cpp

@@ -1 +1 @@
 /**
- * Copyright (C) 2008 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
+ * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  *
  * This library is free software; you can redistribute it and/or
@@ -103 +103 @@
     bool inSameDeck = doc->url().path() == url.path();
     if (inSameDeck && url.hasRef()) {
-        // Force frame loader to load the URL with fragment identifier
-        loader->setForceReloadWmlDeck(true);
-
         if (WMLCardElement* card = WMLCardElement::findNamedCardInDocument(doc, url.ref())) {
             if (card->isNewContext())

trunk/WebCore/wml/WMLPageState.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2008 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
- *
+ * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2004-2007 Apple Inc. All rights reserved.
  *
@@ -26 +25 @@
 #include "WMLPageState.h"
 
+#include "CString.h"
 #include "HistoryItem.h"
 #include "KURL.h"
@@ -34 +34 @@
 WMLPageState::WMLPageState(Page* page)
     : m_page(page)
-    , m_historyLength(0)
     , m_activeCard(0)
     , m_hasDeckAccess(false)
@@ -45 +44 @@
 }
 
+#ifndef NDEBUG
+// Debugging helper for use within gdb
+void WMLPageState::dump()
+{
+    WMLVariableMap::iterator it = m_variables.begin();
+    WMLVariableMap::iterator end = m_variables.end();
+
+    fprintf(stderr, "Dumping WMLPageState (this=%p) associated with Page (page=%p)...\n", this, m_page);
+    for (; it != end; ++it)
+        fprintf(stderr, "\t-> name: '%s'\tvalue: '%s'\n", (*it).first.latin1().data(), (*it).second.latin1().data());
+}
+#endif
+
 void WMLPageState::reset()
 {
-    // remove all the variables in the current browser context
+    // Remove all the variables
     m_variables.clear();
 
-    // clear the navigation history state 
-    if (m_page)
-        m_page->backForwardList()->clearWmlPageHistory();
-
-    // reset implementation-specfic state if UA has
-    m_historyLength = 0;
+    // Clear the navigation history state 
+    if (BackForwardList* list = m_page ? m_page->backForwardList() : 0)
+        list->clearWMLPageHistory();
 }
 

trunk/WebCore/wml/WMLPageState.h

@@ -40 +40 @@
     virtual ~WMLPageState();
 
+#ifndef NDEBUG
+    void dump();
+#endif
+
     // reset the browser context when 'newcontext' attribute
     // of card element is performed as part of go task