Attachment 339328 Details for Bug 185210 – Patch

[patch] Patch

bug-185210-20180502131257.patch (text/plain), 11.17 KB, created by Brent Fulgham on 2018-05-02 13:12:58 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Brent Fulgham

Created: 2018-05-02 13:12:58 PDT

Size: 11.17 KB

patch

obsolete

>Subversion Revision: 231237
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index eeba181eb13501f1d619acf1e95cbc9589821637..bc2a6cd6a807b7a64c06b3399688352e633e8a7e 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2018-05-02  Brent Fulgham  <bfulgham@apple.com>
>+
>+        Use RetainPtr for form input type
>+        https://bugs.webkit.org/show_bug.cgi?id=185210
>+        <rdar://problem/39734040>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Refactor our HTMLInputElement class to store its InputType member as a RefPtr.
>+
>+        Test: fast/forms/access-key-mutation-2.html.
>+
>+        * html/HTMLInputElement.cpp:
>+        (WebCore::HTMLInputElement::HTMLInputElement):
>+        (WebCore::HTMLInputElement::didAddUserAgentShadowRoot):
>+        (WebCore::HTMLInputElement::accessKeyAction):
>+        (WebCore::HTMLInputElement::parseAttribute):
>+        (WebCore::HTMLInputElement::appendFormData):
>+        * html/HTMLInputElement.h:
>+        * html/InputType.cpp:
>+        (WebCore::createInputType):
>+        (WebCore::InputType::create):
>+        (WebCore::InputType::createText):
>+        * html/InputType.h:
>+
> 2018-05-01  Brent Fulgham  <bfulgham@apple.com>
> 
>         Prevent Debug ASSERT when changing forms
>diff --git a/Source/WebCore/html/HTMLInputElement.cpp b/Source/WebCore/html/HTMLInputElement.cpp
>index 7aa71b2c4461c50da0f375a37799a3426676ce1f..429e0ecdf50b2a0c3d0aea1e754372fcda1838bc 100644
>--- a/Source/WebCore/html/HTMLInputElement.cpp
>+++ b/Source/WebCore/html/HTMLInputElement.cpp
>@@ -103,7 +103,6 @@ const int maxSavedResults = 256;
> HTMLInputElement::HTMLInputElement(const QualifiedName& tagName, Document& document, HTMLFormElement* form, bool createdByParser)
>     : HTMLTextFormControlElement(tagName, document, form)
>     , m_size(defaultSize)
>-    , m_maxResults(-1)
>     , m_isChecked(false)
>     , m_reflectsCheckedAttribute(true)
>     , m_isIndeterminate(false)
>@@ -128,7 +127,7 @@ HTMLInputElement::HTMLInputElement(const QualifiedName& tagName, Document& docum
>     , m_isSpellcheckDisabledExceptTextReplacement(false)
>     // m_inputType is lazily created when constructed by the parser to avoid constructing unnecessarily a text inputType and
>     // its shadow subtree, just to destroy them when the |type| attribute gets set by the parser to something else than 'text'.
>-    , m_inputType(createdByParser ? nullptr : InputType::createText(*this))
>+    , m_inputType(createdByParser ? RefPtr<InputType>() : InputType::createText(*this))
> {
>     ASSERT(hasTagName(inputTag));
>     setHasCustomStyleResolveCallbacks();
>@@ -152,7 +151,8 @@ HTMLImageLoader& HTMLInputElement::ensureImageLoader()
> 
> void HTMLInputElement::didAddUserAgentShadowRoot(ShadowRoot&)
> {
>-    m_inputType->createShadowSubtree();
>+    Ref<InputType> protectedInputType(*m_inputType);
>+    protectedInputType->createShadowSubtree();
>     updateInnerTextElementEditability();
> }
> 
>@@ -625,7 +625,8 @@ bool HTMLInputElement::canHaveSelection() const
> 
> void HTMLInputElement::accessKeyAction(bool sendMouseEvents)
> {
>-    m_inputType->accessKeyAction(sendMouseEvents);
>+    Ref<InputType> protectedInputType(*m_inputType);
>+    protectedInputType->accessKeyAction(sendMouseEvents);
> }
> 
> bool HTMLInputElement::isPresentationAttribute(const QualifiedName& name) const
>@@ -682,6 +683,7 @@ inline void HTMLInputElement::initializeInputType()
> void HTMLInputElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
> {
>     ASSERT(m_inputType);
>+    Ref<InputType> protectedInputType(*m_inputType);
> 
>     if (name == nameAttr) {
>         removeFromRadioButtonGroup();
>@@ -889,6 +891,7 @@ void HTMLInputElement::setActivatedSubmit(bool flag)
> 
> bool HTMLInputElement::appendFormData(DOMFormData& formData, bool multipart)
> {
>+    Ref<InputType> protectedInputType(*m_inputType);
>     return m_inputType->isFormDataAppendable() && m_inputType->appendFormData(formData, multipart);
> }
> 
>diff --git a/Source/WebCore/html/HTMLInputElement.h b/Source/WebCore/html/HTMLInputElement.h
>index f8d9d2c7509093de0489eb610602157de645df86..138433241fa549453f69055f1675b476a69c9465 100644
>--- a/Source/WebCore/html/HTMLInputElement.h
>+++ b/Source/WebCore/html/HTMLInputElement.h
>@@ -447,7 +447,7 @@ private:
>     AtomicString m_name;
>     String m_valueIfDirty;
>     unsigned m_size;
>-    short m_maxResults;
>+    short m_maxResults { -1 };
>     bool m_isChecked : 1;
>     bool m_reflectsCheckedAttribute : 1;
>     bool m_isIndeterminate : 1;
>@@ -470,7 +470,7 @@ private:
>     bool m_hasTouchEventHandler : 1;
> #endif
>     bool m_isSpellcheckDisabledExceptTextReplacement : 1;
>-    std::unique_ptr<InputType> m_inputType;
>+    RefPtr<InputType> m_inputType;
>     // The ImageLoader must be owned by this element because the loader code assumes
>     // that it lives as long as its owning element lives. If we move the loader into
>     // the ImageInput object we may delete the loader while this element lives on.
>diff --git a/Source/WebCore/html/InputType.cpp b/Source/WebCore/html/InputType.cpp
>index b92a6569fd148a43494c330c2ec0aca7a5188849..8bec3a6f400974aadc4beab349be06f8d4cb7a57 100644
>--- a/Source/WebCore/html/InputType.cpp
>+++ b/Source/WebCore/html/InputType.cpp
>@@ -2,7 +2,7 @@
>  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
>  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
>  *           (C) 2001 Dirk Mueller (mueller@kde.org)
>- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2004-2018 Apple Inc. All rights reserved.
>  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
>  * Copyright (C) 2007 Samuel Weinig (sam@webkit.org)
>  * Copyright (C) 2009, 2010, 2011, 2012 Google Inc. All rights reserved.
>@@ -85,13 +85,13 @@ using namespace HTMLNames;
> 
> typedef bool (RuntimeEnabledFeatures::*InputTypeConditionalFunction)() const;
> typedef const AtomicString& (*InputTypeNameFunction)();
>-typedef std::unique_ptr<InputType> (*InputTypeFactoryFunction)(HTMLInputElement&);
>+typedef Ref<InputType> (*InputTypeFactoryFunction)(HTMLInputElement&);
> typedef HashMap<AtomicString, InputTypeFactoryFunction, ASCIICaseInsensitiveHash> InputTypeFactoryMap;
> 
> template<class T>
>-static std::unique_ptr<InputType> createInputType(HTMLInputElement& element)
>+static Ref<InputType> createInputType(HTMLInputElement& element)
> {
>-    return std::make_unique<T>(element);
>+    return adoptRef(*new T(element));
> }
> 
> static InputTypeFactoryMap createInputTypeFactoryMap()
>@@ -149,19 +149,19 @@ static InputTypeFactoryMap createInputTypeFactoryMap()
>     return map;
> }
> 
>-std::unique_ptr<InputType> InputType::create(HTMLInputElement& element, const AtomicString& typeName)
>+Ref<InputType> InputType::create(HTMLInputElement& element, const AtomicString& typeName)
> {
>     if (!typeName.isEmpty()) {
>         static const auto factoryMap = makeNeverDestroyed(createInputTypeFactoryMap());
>         if (auto factory = factoryMap.get().get(typeName))
>             return factory(element);
>     }
>-    return std::make_unique<TextInputType>(element);
>+    return adoptRef(*new TextInputType(element));
> }
> 
>-std::unique_ptr<InputType> InputType::createText(HTMLInputElement& element)
>+Ref<InputType> InputType::createText(HTMLInputElement& element)
> {
>-    return std::make_unique<TextInputType>(element);
>+    return adoptRef(*new TextInputType(element));
> }
> 
> InputType::~InputType() = default;
>diff --git a/Source/WebCore/html/InputType.h b/Source/WebCore/html/InputType.h
>index 5268d8c52bef17326943bdf4ebdd8e16a8d1ed77..d0b86b58a035bbc01546ba3c4712fa40661413b0 100644
>--- a/Source/WebCore/html/InputType.h
>+++ b/Source/WebCore/html/InputType.h
>@@ -1,6 +1,6 @@
> /*
>  * Copyright (C) 2010 Google Inc. All rights reserved.
>- * Copyright (C) 2011-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
>  * Copyright (C) 2012 Samsung Electronics. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>@@ -37,6 +37,7 @@
> #include "StepRange.h"
> #include <wtf/FastMalloc.h>
> #include <wtf/Forward.h>
>+#include <wtf/RefCounted.h>
> #include <wtf/RefPtr.h>
> 
> #if PLATFORM(IOS)
>@@ -68,12 +69,12 @@ struct InputElementClickState;
> // An InputType object represents the type-specific part of an HTMLInputElement.
> // Do not expose instances of InputType and classes derived from it to classes
> // other than HTMLInputElement.
>-class InputType {
>+class InputType : public RefCounted<InputType> {
>     WTF_MAKE_FAST_ALLOCATED;
> 
> public:
>-    static std::unique_ptr<InputType> create(HTMLInputElement&, const AtomicString&);
>-    static std::unique_ptr<InputType> createText(HTMLInputElement&);
>+    static Ref<InputType> create(HTMLInputElement&, const AtomicString&);
>+    static Ref<InputType> createText(HTMLInputElement&);
>     virtual ~InputType();
> 
>     static bool themeSupportsDataListUI(InputType*);
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 4f8b77fd676156e827ec46a92ffb9b9246f21946..690452c5dfc0d6b794d1852fcfe74aa9624c551c 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-05-02  Brent Fulgham  <bfulgham@apple.com>
>+
>+        Use RetainPtr for form input type
>+        https://bugs.webkit.org/show_bug.cgi?id=185210
>+        <rdar://problem/39734040>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/forms/access-key-mutation-2-expected.txt: Added.
>+        * fast/forms/access-key-mutation-2.html: Added.
>+
> 2018-05-01  Brent Fulgham  <bfulgham@apple.com>
> 
>         Prevent assertion when changing forms
>diff --git a/LayoutTests/fast/forms/access-key-mutation-2-expected.txt b/LayoutTests/fast/forms/access-key-mutation-2-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..b3e9cf4bcfc2759510087a7982ac7fb6ebd87526
>--- /dev/null
>+++ b/LayoutTests/fast/forms/access-key-mutation-2-expected.txt
>@@ -0,0 +1,10 @@
>+Access key should work when input type attribute is mutated. To test this manually, press the <alt>+k keys (on Mac OS X, press <Ctrl>+<Opt> instead of <alt>).
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS if not crashed.
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+ 
>diff --git a/LayoutTests/fast/forms/access-key-mutation-2.html b/LayoutTests/fast/forms/access-key-mutation-2.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..53291da14571cddb6f558d9d49939c1765c4311a
>--- /dev/null
>+++ b/LayoutTests/fast/forms/access-key-mutation-2.html
>@@ -0,0 +1,39 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../../resources/js-test.js"></script>
>+<script>
>+description('Access key should work when input type attribute is mutated. To test this manually, press the &lt;alt&gt;+k keys (on Mac OS X, press &lt;Ctrl&gt;+&lt;Opt&gt; instead of &lt;alt&gt;).');
>+
>+jsTestIsAsync = true;
>+
>+function pressKey(key)
>+{
>+    if (navigator.userAgent.search(/\bMac OS X\b/) !== -1)
>+        modifiers = ["ctrlKey", "altKey"];
>+    else
>+        modifiers = ["altKey"];
>+
>+    if (window.eventSender)
>+        eventSender.keyDown(key, modifiers);
>+}
>+
>+function eventhandler()
>+{
>+    input.type = "button"
>+}
>+
>+function start()
>+{
>+    pressKey('k');
>+    testPassed('if not crashed.');
>+    finishJSTest();
>+}
>+</script>
>+</head>
>+<body>
>+<input id="input" type="range" accesskey="k" onfocus="eventhandler()">
>+<iframe onload="start()"></iframe>
>+</body>
>+</html>
>+

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185210: 339328