Attachment 122518 Details for Bug 76035 – new try based on brady's suggestions

[patch] new try based on brady's suggestions

bug-76035-20120113161156.patch (text/plain), 16.85 KB, created by Pablo Flouret on 2012-01-13 16:11:57 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Pablo Flouret

Created: 2012-01-13 16:11:57 PST

Size: 16.85 KB

patch

obsolete

>Subversion Revision: 104981
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 1a8f1315499983d69391fec4df98a38559a3893d..3f0c1c7b64801c87a85e0a87ff1d87e7e45469fc 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,5 +1,31 @@
> 2012-01-13  Pablo Flouret  <pablof@motorola.com>
> 
>+        Add state attribute to history's dom interface.
>+        https://bugs.webkit.org/show_bug.cgi?id=76035
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Tests: fast/loader/stateobjects/pushstate-state-attribute-object-types.html
>+               fast/loader/stateobjects/pushstate-state-attribute-only-one-deserialization.html
>+
>+        * bindings/js/JSHistoryCustom.cpp:
>+        (WebCore::JSHistory::state):
>+        * bindings/v8/custom/V8HistoryCustom.cpp:
>+        * history/HistoryItem.cpp:
>+        (WebCore::HistoryItem::reset):
>+        (WebCore::HistoryItem::setURL):
>+        (WebCore::HistoryItem::setStateObject):
>+        (WebCore::HistoryItem::clearDeserializedStateObject):
>+        * history/HistoryItem.h:
>+        (WebCore::HistoryItem::deserializedStateObject):
>+        * page/History.cpp:
>+        (WebCore::History::state):
>+        * page/History.h:
>+        * page/History.idl:
>+
>+
>+2012-01-13  Pablo Flouret  <pablof@motorola.com>
>+
>         Fix compilation errors on build-webkit --debug --no-video on mac.
>         https://bugs.webkit.org/show_bug.cgi?id=75867
> 
>diff --git a/Source/WebCore/bindings/js/JSHistoryCustom.cpp b/Source/WebCore/bindings/js/JSHistoryCustom.cpp
>index 72ea5424c7c12f3a672bc7d739e11dedfb4818c7..47f3db0da93e7a908ce8a9078f800089746cc8d8 100644
>--- a/Source/WebCore/bindings/js/JSHistoryCustom.cpp
>+++ b/Source/WebCore/bindings/js/JSHistoryCustom.cpp
>@@ -31,6 +31,7 @@
> 
> #include "Frame.h"
> #include "History.h"
>+#include "ScriptValue.h"
> #include "SerializedScriptValue.h"
> #include <runtime/JSFunction.h>
> 
>@@ -164,6 +165,13 @@ void JSHistory::getOwnPropertyNames(JSObject* object, ExecState* exec, PropertyN
>     Base::getOwnPropertyNames(thisObject, exec, propertyNames, mode);
> }
> 
>+JSValue JSHistory::state(ExecState* exec) const
>+{
>+    History* history = static_cast<History*>(impl());
>+    ScriptValue state = history->state(exec);
>+    return state.hasNoValue() ? jsNull() : state.jsValue();
>+}
>+
> JSValue JSHistory::pushState(ExecState* exec)
> {
>     RefPtr<SerializedScriptValue> historyState = SerializedScriptValue::create(exec, exec->argument(0), 0);
>diff --git a/Source/WebCore/bindings/v8/custom/V8HistoryCustom.cpp b/Source/WebCore/bindings/v8/custom/V8HistoryCustom.cpp
>index 89561090ca149dbd1ef7036a91cd3a9a7efba68b..9ff967263c48f8c53e26426d17562e63efbb730b 100644
>--- a/Source/WebCore/bindings/v8/custom/V8HistoryCustom.cpp
>+++ b/Source/WebCore/bindings/v8/custom/V8HistoryCustom.cpp
>@@ -33,6 +33,7 @@
> 
> #include "ExceptionCode.h"
> #include "History.h"
>+#include "ScriptValue.h"
> #include "SerializedScriptValue.h"
> #include "V8Binding.h"
> #include "V8BindingState.h"
>diff --git a/Source/WebCore/history/HistoryItem.cpp b/Source/WebCore/history/HistoryItem.cpp
>index 21c81ede7efe261ad869751dcf9e8f2a40a5e892..9f3b32a3756a22a159019e4b0907f562e5ed6b97 100644
>--- a/Source/WebCore/history/HistoryItem.cpp
>+++ b/Source/WebCore/history/HistoryItem.cpp
>@@ -31,6 +31,7 @@
> #include "IconDatabase.h"
> #include "PageCache.h"
> #include "ResourceRequest.h"
>+#include "ScriptValue.h"
> #include "SerializedScriptValue.h"
> #include "SharedBuffer.h"
> #include <stdio.h>
>@@ -200,6 +201,7 @@ void HistoryItem::reset()
>     m_itemSequenceNumber = generateSequenceNumber();
> 
>     m_stateObject = 0;
>+    clearDeserializedStateObject();
>     m_documentSequenceNumber = generateSequenceNumber();
> 
>     m_formData = 0;
>@@ -282,6 +284,7 @@ void HistoryItem::setURL(const KURL& url)
>     pageCache()->remove(this);
>     setURLString(url.string());
>     clearDocumentState();
>+    clearDeserializedStateObject();
> }
> 
> void HistoryItem::setOriginalURLString(const String& urlString)
>@@ -456,6 +459,13 @@ void HistoryItem::setIsTargetItem(bool flag)
> void HistoryItem::setStateObject(PassRefPtr<SerializedScriptValue> object)
> {
>     m_stateObject = object;
>+    clearDeserializedStateObject();
>+}
>+
>+void HistoryItem::clearDeserializedStateObject()
>+{
>+    if (!m_deserializedStateObject.hasNoValue())
>+        m_deserializedStateObject = ScriptValue();
> }
> 
> void HistoryItem::addChildItem(PassRefPtr<HistoryItem> child)
>diff --git a/Source/WebCore/history/HistoryItem.h b/Source/WebCore/history/HistoryItem.h
>index 8619d0c346bf86b9e729269629d3825b702f6b73..a85445b3c49c01cd10f03aa14abd00c330ab2b59 100644
>--- a/Source/WebCore/history/HistoryItem.h
>+++ b/Source/WebCore/history/HistoryItem.h
>@@ -28,6 +28,7 @@
> 
> #include "IntPoint.h"
> #include "PlatformString.h"
>+#include "ScriptValue.h"
> #include <wtf/HashMap.h>
> #include <wtf/OwnPtr.h>
> #include <wtf/PassOwnPtr.h>
>@@ -53,6 +54,7 @@ class HistoryItem;
> class Image;
> class KURL;
> class ResourceRequest;
>+class ScriptValue;
> class SerializedScriptValue;
> 
> typedef Vector<RefPtr<HistoryItem> > HistoryItemVector;
>@@ -141,6 +143,7 @@ public:
>     
>     void setStateObject(PassRefPtr<SerializedScriptValue> object);
>     SerializedScriptValue* stateObject() const { return m_stateObject.get(); }
>+    ScriptValue& deserializedStateObject() { return m_deserializedStateObject; }
> 
>     void setItemSequenceNumber(long long number) { m_itemSequenceNumber = number; }
>     long long itemSequenceNumber() const { return m_itemSequenceNumber; }
>@@ -228,6 +231,8 @@ private:
> 
>     void encodeBackForwardTreeNode(Encoder&) const;
> 
>+    void clearDeserializedStateObject();
>+
>     /* When adding new member variables to this class, please notify the Qt team.
>      * qt/HistoryItemQt.cpp contains code to serialize history items.
>      */
>@@ -270,6 +275,7 @@ private:
> 
>     // Support for HTML5 History
>     RefPtr<SerializedScriptValue> m_stateObject;
>+    ScriptValue m_deserializedStateObject;
>     
>     // info used to repost form data
>     RefPtr<FormData> m_formData;
>diff --git a/Source/WebCore/page/History.cpp b/Source/WebCore/page/History.cpp
>index 1f5a39f287584f7c2748b33e1099e664d192ae68..4dec262699246d782ab8994c943c8bb57625ffa8 100644
>--- a/Source/WebCore/page/History.cpp
>+++ b/Source/WebCore/page/History.cpp
>@@ -32,8 +32,10 @@
> #include "Frame.h"
> #include "FrameLoader.h"
> #include "FrameLoaderClient.h"
>+#include "HistoryController.h"
> #include "HistoryItem.h"
> #include "Page.h"
>+#include "ScriptState.h"
> #include "SecurityOrigin.h"
> #include "SerializedScriptValue.h"
> #include <wtf/MainThread.h>
>@@ -107,6 +109,26 @@ KURL History::urlForState(const String& urlString)
>     return KURL(baseURL, urlString);
> }
> 
>+ScriptValue History::state(ScriptState* exec) const
>+{
>+    if (m_frame) {
>+        if (HistoryItem* historyItem = m_frame->loader()->history()->currentItem()) {
>+            ScriptValue& state = historyItem->deserializedStateObject();
>+            if (state.hasNoValue() && historyItem->stateObject()) {
>+#if USE(V8)
>+                state = ScriptValue(historyItem->stateObject()->deserialize());
>+#else
>+                ASSERT(exec);
>+                state = ScriptValue::deserialize(exec, historyItem->stateObject());
>+#endif
>+            }
>+            return state;
>+        }
>+    }
>+
>+    return ScriptValue();
>+}
>+
> void History::stateObjectAdded(PassRefPtr<SerializedScriptValue> data, const String& title, const String& urlString, StateObjectType stateObjectType, ExceptionCode& ec)
> {
>     if (!m_frame || !m_frame->page())
>diff --git a/Source/WebCore/page/History.h b/Source/WebCore/page/History.h
>index bd50fccd2b861d9e6418e974f42dfb2b2b5f91ce..754cf92049255351dec0d21cb86e6bf338403268 100644
>--- a/Source/WebCore/page/History.h
>+++ b/Source/WebCore/page/History.h
>@@ -28,6 +28,7 @@
> 
> #include "DOMWindowProperty.h"
> #include "KURL.h"
>+#include "ScriptState.h"
> #include <wtf/Forward.h>
> #include <wtf/PassRefPtr.h>
> #include <wtf/RefCounted.h>
>@@ -36,6 +37,7 @@ namespace WebCore {
> 
> class Frame;
> class ScriptExecutionContext;
>+class ScriptValue;
> class SerializedScriptValue;
> typedef int ExceptionCode;
> 
>@@ -56,6 +58,9 @@ public:
>         StateObjectPush,
>         StateObjectReplace
>     };
>+
>+    ScriptValue state(ScriptState* exec = 0) const;
>+
>     void stateObjectAdded(PassRefPtr<SerializedScriptValue>, const String& title, const String& url, StateObjectType, ExceptionCode&);
> 
> private:
>diff --git a/Source/WebCore/page/History.idl b/Source/WebCore/page/History.idl
>index ff3701ed276d4f5698da6a0530e066380a33b1c1..f0b65668654c5e1c6ce2fe8f96ff17e72f9f9ed1 100644
>--- a/Source/WebCore/page/History.idl
>+++ b/Source/WebCore/page/History.idl
>@@ -37,6 +37,7 @@ module window {
>         OmitConstructor
>     ] History {
>         readonly attribute unsigned long length;
>+        readonly attribute [JSCCustomGetter] DOMObject state;
> 
>         [DoNotCheckDomainSecurity, CallWith=ScriptExecutionContext] void back();
>         [DoNotCheckDomainSecurity, CallWith=ScriptExecutionContext] void forward();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 1037d0a3587ce0d0afb651da1b9a38bd54578332..126ec45fa5d98e2e47a02bc416d5fe4c75d4ec99 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2012-01-13  Pablo Flouret  <pablof@motorola.com>
>+
>+        Add state attribute to history's dom interface.
>+        https://bugs.webkit.org/show_bug.cgi?id=76035
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/loader/stateobjects/pushstate-state-attribute-object-types-expected.txt: Added.
>+        * fast/loader/stateobjects/pushstate-state-attribute-object-types.html: Added.
>+        * fast/loader/stateobjects/pushstate-state-attribute-only-one-deserialization-expected.txt: Added.
>+        * fast/loader/stateobjects/pushstate-state-attribute-only-one-deserialization.html: Added.
>+
> 2012-01-13  Simon Fraser  <simon.fraser@apple.com>
> 
>         Add new baselines for various compositing tests which didn't have Mac results.
>diff --git a/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-object-types-expected.txt b/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-object-types-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..c1a87a78fbc6893f0c6e1f99584cedb53eb86611
>--- /dev/null
>+++ b/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-object-types-expected.txt
>@@ -0,0 +1,15 @@
>+This test calls pushState with state objects of all the different object types supported by the HTML5 "internal structured cloning algorithm" and makes sure the state attribute returns the expected objects.
>+
>+State is initially null (type object)
>+State pushed - undefined (type undefined)
>+State pushed - null (type object)
>+State pushed - false (type boolean)
>+State pushed - true (type boolean)
>+State pushed - 42 (type number)
>+State pushed - String (type string)
>+State pushed - 0 (type object)
>+State pushed - /foo/gi (type object)
>+State pushed -  (type object)
>+State pushed - [object Object] (type object)
>+State pushed - [object ImageData] (type object)
>+
>diff --git a/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-object-types.html b/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-object-types.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..f35a3d79336178bc2bc4a02f2e289903f4f9b074
>--- /dev/null
>+++ b/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-object-types.html
>@@ -0,0 +1,57 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script>
>+
>+if (window.layoutTestController) {
>+    layoutTestController.clearBackForwardList();
>+    layoutTestController.dumpAsText();
>+}
>+
>+function log(txt)
>+{
>+    document.getElementById("logger").appendChild(document.createTextNode(txt + "\n"));
>+}
>+
>+function runTest()
>+{
>+    log("State is initially " + history.state + " (type " + typeof history.state + ")");
>+    pushAndTestState(undefined, "undefined entry");
>+    pushAndTestState(null, "null entry");
>+    pushAndTestState(false, "false entry");
>+    pushAndTestState(true, "true entry");
>+    pushAndTestState(42, "Number entry");
>+    pushAndTestState("String", "String entry");
>+    pushAndTestState(new Date(0), "Date entry");
>+    pushAndTestState(new RegExp("foo", "gi"), "RegExp entry");
>+    pushAndTestState(new Array, "Array entry");
>+    pushAndTestState(new Object, "Object entry");
>+    pushAndTestState(document.createElement("canvas").getContext("2d").createImageData(10,10), "ImageData entry");
>+
>+/* The following are not yet well enough supported in WebKit to meaningfully test here:
>+    history.pushAndState([Blob], "Blob entry");
>+    history.pushAndState([File], "File entry");
>+    history.pushAndState([FileList], "FileList entry");
>+*/
>+}
>+
>+function pushAndTestState(data, title)
>+{
>+    history.pushState(data, title);
>+    var state = history.state;
>+
>+    if (state instanceof Date)
>+        log("State pushed - " + state.getTime() + " (type " + typeof state + ")");
>+    else
>+        log("State pushed - " + state + " (type " + typeof state + ")");
>+}
>+
>+</script>
>+<body onload="runTest()">
>+<p>
>+This test calls pushState with state objects of all the different object types supported by the
>+HTML5 "internal structured cloning algorithm" and makes sure the state attribute returns the expected objects.
>+</p>
>+<pre id="logger"></pre>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-only-one-deserialization-expected.txt b/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-only-one-deserialization-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..1fd75d7fa0805966b5cd8eb3299adbd303102e72
>--- /dev/null
>+++ b/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-only-one-deserialization-expected.txt
>@@ -0,0 +1,17 @@
>+Make sure the same deserialization of the state object is used every time (both in the history object and popstate events).
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS history.state !== stateObject is true
>+PASS history.state === stateObject is true
>+
>+Inside popstate event
>+
>+PASS history.state !== stateObject is true
>+PASS event.state !== stateObject is true
>+FAIL event.state === history.state should be true. Was false.
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-only-one-deserialization.html b/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-only-one-deserialization.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..5ebebcf63e52ddb0105af2174be3456d5340fd84
>--- /dev/null
>+++ b/LayoutTests/fast/loader/stateobjects/pushstate-state-attribute-only-one-deserialization.html
>@@ -0,0 +1,60 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <script src="../../js/resources/js-test-pre.js"></script>
>+</head/>
>+<body>
>+<p id=description>
>+</p>
>+<div id="console"></div>
>+<script>
>+    description("Make sure the same deserialization of the state object is used every time (both in the history object and popstate events).");
>+
>+    if (window.layoutTestController) {
>+        layoutTestController.clearBackForwardList();
>+        layoutTestController.dumpAsText();
>+        layoutTestController.waitUntilDone();
>+    }
>+
>+    window.jsTestIsAsync = true;
>+
>+    // Create a new object.
>+    var stateObject = ["a"];
>+
>+    // Use it as the state object in a replaceState.  This clones the object.
>+    history.replaceState(stateObject, null, null);
>+
>+    // Since the actual history.state is a structured clone, it should not match our original object.
>+    shouldBeTrue("history.state !== stateObject");
>+
>+    // Now let's refetch a copy of history.state to store;
>+    stateObject = history.state;
>+
>+    // Our reference and the history.state itself should be the same.  This is now Adam's original assertion.
>+    shouldBeTrue("history.state === stateObject");
>+
>+    // Now let's do a pushstate to add a history entry.
>+    history.pushState(null, null, null);
>+
>+    // Now add a handler for the popstate event.
>+    window.onpopstate = function(event) {
>+        debug("\nInside popstate event\n");
>+        // Our stored reference to stateObject will not match the current state object, as it is a structured clone of the history item's state object.
>+        shouldBeTrue("history.state !== stateObject");
>+        // Our stored reference to stateObject will not match the state object in this pop state event, as it is the same as history.state which is a structured clone of the history item's state object.
>+        shouldBeTrue("event.state !== stateObject");
>+        // The event's state object and the current state object should match.
>+        shouldBeTrue("event.state === history.state"); // This fails for now, needs to be fixed.
>+
>+        var s = document.createElement("script");
>+        s.src = "../../js/resources/js-test-post.js";
>+        document.body.appendChild(s);
>+        setTimeout(finishJSTest, 0);
>+    }
>+
>+    // Now let's go back to our original history entry which has a state object that we've stored a reference to already.
>+    // This will fire our popstate event handler above.
>+    history.back();
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 76035: 121987 | 122471 | 122518 | 122586 | 122857 | 122860 | 123150 | 124811 | 124856 | 125274 | 126022