Get rid of custom bindings for History's replaceState() / pushState()
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Mar 2018 03:31:01 +0000 (03:31 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Mar 2018 03:31:01 +0000 (03:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183372

Reviewed by Youenn Fablet.

Get rid of custom bindings for History's replaceState() / pushState() by
moving the cached state from the wrapper to the History implementation
object.

No new tests, no web-facing behavior change.

* bindings/js/JSHistoryCustom.cpp:
(WebCore::JSHistory::state const):
(WebCore::JSHistory::visitAdditionalChildren):
* page/History.cpp:
(WebCore::History::cachedState):
(WebCore::History::stateObjectAdded):
* page/History.h:
(WebCore::History::pushState):
(WebCore::History::replaceState):
* page/History.idl:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229392 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSHistoryCustom.cpp
Source/WebCore/page/History.cpp
Source/WebCore/page/History.h
Source/WebCore/page/History.idl

index 727aa25..d72178a 100644 (file)
@@ -1,3 +1,27 @@
+2018-03-07  Chris Dumez  <cdumez@apple.com>
+
+        Get rid of custom bindings for History's replaceState() / pushState()
+        https://bugs.webkit.org/show_bug.cgi?id=183372
+
+        Reviewed by Youenn Fablet.
+
+        Get rid of custom bindings for History's replaceState() / pushState() by
+        moving the cached state from the wrapper to the History implementation
+        object.
+
+        No new tests, no web-facing behavior change.
+
+        * bindings/js/JSHistoryCustom.cpp:
+        (WebCore::JSHistory::state const):
+        (WebCore::JSHistory::visitAdditionalChildren):
+        * page/History.cpp:
+        (WebCore::History::cachedState):
+        (WebCore::History::stateObjectAdded):
+        * page/History.h:
+        (WebCore::History::pushState):
+        (WebCore::History::replaceState):
+        * page/History.idl:
+
 2018-03-07  Youenn Fablet  <youenn@apple.com>
 
         Match unsupported plugins based on domains and not origin
index 55d5067..514c0ed 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 #include "config.h"
 #include "JSHistory.h"
 
-#include "Frame.h"
-#include "JSDOMConvertNullable.h"
-#include "JSDOMConvertStrings.h"
 #include "SerializedScriptValue.h"
-#include <JavaScriptCore/JSFunction.h>
 
 namespace WebCore {
+
 using namespace JSC;
 
 JSValue JSHistory::state(ExecState& state) const
 {
-    History& history = wrapped();
-
-    JSValue cachedValue = m_state.get();
-    if (!cachedValue.isEmpty() && !history.stateChanged())
-        return cachedValue;
-
-    RefPtr<SerializedScriptValue> serialized = history.state();
-    JSValue result = serialized ? serialized->deserialize(state, globalObject()) : jsNull();
-    m_state.set(state.vm(), this, result);
-    return result;
-}
-
-JSValue JSHistory::pushState(ExecState& state)
-{
-    VM& vm = state.vm();
-    auto scope = DECLARE_THROW_SCOPE(vm);
-
-    auto argCount = state.argumentCount();
-    if (UNLIKELY(argCount < 2))
-        return throwException(&state, scope, createNotEnoughArgumentsError(&state));
-
-    auto historyState = SerializedScriptValue::create(state, state.uncheckedArgument(0));
-    RETURN_IF_EXCEPTION(scope, JSValue());
-
-    // FIXME: title should not be nullable.
-    String title = convert<IDLNullable<IDLDOMString>>(state, state.uncheckedArgument(1));
-    RETURN_IF_EXCEPTION(scope, JSValue());
-
-    String url;
-    if (argCount > 2) {
-        url = convert<IDLNullable<IDLUSVString>>(state, state.uncheckedArgument(2));
-        RETURN_IF_EXCEPTION(scope, JSValue());
-    }
-
-    propagateException(state, scope, wrapped().stateObjectAdded(WTFMove(historyState), title, url, History::StateObjectType::Push));
-
-    m_state.clear();
-
-    return jsUndefined();
+    return cachedPropertyValue(state, *this, wrapped().cachedState(), [this, &state] {
+        auto* serialized = wrapped().state();
+        return serialized ? serialized->deserialize(state, globalObject()) : jsNull();
+    });
 }
 
-JSValue JSHistory::replaceState(ExecState& state)
+void JSHistory::visitAdditionalChildren(SlotVisitor& visitor)
 {
-    VM& vm = state.vm();
-    auto scope = DECLARE_THROW_SCOPE(vm);
-
-    auto argCount = state.argumentCount();
-    if (UNLIKELY(argCount < 2))
-        return throwException(&state, scope, createNotEnoughArgumentsError(&state));
-
-    auto historyState = SerializedScriptValue::create(state, state.uncheckedArgument(0));
-    RETURN_IF_EXCEPTION(scope, JSValue());
-
-    // FIXME: title should not be nullable.
-    String title = convert<IDLNullable<IDLDOMString>>(state, state.uncheckedArgument(1));
-    RETURN_IF_EXCEPTION(scope, JSValue());
-
-    String url;
-    if (argCount > 2) {
-        url = convert<IDLNullable<IDLUSVString>>(state, state.uncheckedArgument(2));
-        RETURN_IF_EXCEPTION(scope, JSValue());
-    }
-
-    propagateException(state, scope, wrapped().stateObjectAdded(WTFMove(historyState), title, url, History::StateObjectType::Replace));
-
-    m_state.clear();
-
-    return jsUndefined();
+    wrapped().cachedState().visit(visitor);
 }
 
 } // namespace WebCore
index 9826b8b..ad48313 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007 Apple Inc.  All rights reserved.
+ * Copyright (C) 2007-2018 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -104,6 +104,13 @@ bool History::stateChanged() const
     return m_lastStateObjectRequested != stateInternal();
 }
 
+JSValueInWrappedObject& History::cachedState()
+{
+    if (m_cachedState && stateChanged())
+        m_cachedState = { };
+    return m_cachedState;
+}
+
 bool History::isSameAsCurrentState(SerializedScriptValue* state) const
 {
     return state == stateInternal();
@@ -163,6 +170,8 @@ URL History::urlForState(const String& urlString)
 
 ExceptionOr<void> History::stateObjectAdded(RefPtr<SerializedScriptValue>&& data, const String& title, const String& urlString, StateObjectType stateObjectType)
 {
+    m_cachedState = { };
+
     // Each unique main-frame document is only allowed to send 64MB of state object payload to the UI client/process.
     static uint32_t totalStateObjectPayloadLimit = 0x4000000;
     static Seconds stateObjectTimeSpan { 30_s };
index 8d83275..05fe2b3 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007 Apple Inc.  All rights reserved.
+ * Copyright (C) 2007-2018 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -27,6 +27,7 @@
 
 #include "DOMWindowProperty.h"
 #include "ExceptionOr.h"
+#include "JSValueInWrappedObject.h"
 #include "ScriptWrappable.h"
 #include "SerializedScriptValue.h"
 #include <wtf/WallTime.h>
@@ -52,6 +53,8 @@ public:
     ExceptionOr<void> setScrollRestoration(ScrollRestoration);
 
     SerializedScriptValue* state();
+    JSValueInWrappedObject& cachedState();
+
     void back();
     void forward();
     void go(int);
@@ -60,20 +63,24 @@ public:
     void forward(Document&);
     void go(Document&, int);
 
-    bool stateChanged() const;
     bool isSameAsCurrentState(SerializedScriptValue*) const;
 
-    enum class StateObjectType { Push, Replace };
-    ExceptionOr<void> stateObjectAdded(RefPtr<SerializedScriptValue>&&, const String& title, const String& url, StateObjectType);
+    ExceptionOr<void> pushState(RefPtr<SerializedScriptValue>&& data, const String& title, const String& urlString);
+    ExceptionOr<void> replaceState(RefPtr<SerializedScriptValue>&& data, const String& title, const String& urlString);
 
 private:
     explicit History(Frame&);
 
+    enum class StateObjectType { Push, Replace };
+    ExceptionOr<void> stateObjectAdded(RefPtr<SerializedScriptValue>&&, const String& title, const String& url, StateObjectType);
+    bool stateChanged() const;
+
     URL urlForState(const String& url);
 
     SerializedScriptValue* stateInternal() const;
 
     RefPtr<SerializedScriptValue> m_lastStateObjectRequested;
+    JSValueInWrappedObject m_cachedState;
 
     unsigned m_currentStateObjectTimeSpanObjectsAdded { 0 };
     WallTime m_currentStateObjectTimeSpanStart;
@@ -85,4 +92,14 @@ private:
     uint64_t m_mostRecentStateObjectUsage { 0 };
 };
 
+inline ExceptionOr<void> History::pushState(RefPtr<SerializedScriptValue>&& data, const String& title, const String& urlString)
+{
+    return stateObjectAdded(WTFMove(data), title, urlString, StateObjectType::Push);
+}
+
+inline ExceptionOr<void> History::replaceState(RefPtr<SerializedScriptValue>&& data, const String& title, const String& urlString)
+{
+    return stateObjectAdded(WTFMove(data), title, urlString, StateObjectType::Replace);
+}
+
 } // namespace WebCore
index 0a3feca..33a80fb 100644 (file)
 
 [
     GenerateIsReachable=ImplFrame,
+    JSCustomMarkFunction,
 ] interface History {
     readonly attribute unsigned long length;
     attribute ScrollRestoration scrollRestoration;
-    [CachedAttribute, Custom] readonly attribute SerializedScriptValue state;
+    [Custom] readonly attribute any state;
 
     [CallWith=Document, ForwardDeclareInHeader] void back();
     [CallWith=Document, ForwardDeclareInHeader] void forward();
-    [CallWith=Document, ForwardDeclareInHeader] void go(optional long distance = 0);
+    [CallWith=Document, ForwardDeclareInHeader] void go(optional long delta = 0);
 
-    [Custom, MayThrowException] void pushState(any data, DOMString title, optional USVString? url = null);
-    [Custom, MayThrowException] void replaceState(any data, DOMString title, optional USVString? url = null);
+    // FIXME: title should not be nullable as per the HTML specification.
+    [MayThrowException] void pushState(SerializedScriptValue data, DOMString? title, optional USVString? url = null);
+    [MayThrowException] void replaceState(SerializedScriptValue data, DOMString? title, optional USVString? url = null);
 };
 
 enum ScrollRestoration { "auto", "manual" };