Change ColorChooser from singleton to ordinary object
authorkeishi@webkit.org <keishi@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 Nov 2011 11:39:32 +0000 (11:39 +0000)
committerkeishi@webkit.org <keishi@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 Nov 2011 11:39:32 +0000 (11:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=71644

Reviewed by Kent Tamura.

Source/WebCore:

Changing WebCore::ColorChooser from a singleton to an ordinary object can broaden how browsers implement the color chooser interface.

* WebCore.exp.in:
* html/ColorInputType.cpp:
(WebCore::ColorInputType::~ColorInputType):
(WebCore::ColorInputType::setValue): If a chooser exists, calls Chrome::setSelectedColorInColorChooser
(WebCore::ColorInputType::handleDOMActivateEvent):
(WebCore::ColorInputType::detach):
(WebCore::ColorInputType::didCleanup): Called after cleanup is complete.
(WebCore::ColorInputType::cleanupColorChooser): Renamed from cleanupColorChooserIfCurrentClient.
* html/ColorInputType.h:
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::selectColorInColorChooser):
* html/HTMLInputElement.h:
* loader/EmptyClients.h:
(WebCore::EmptyChromeClient::cleanupColorChooser): Added colorChooser argument because there are many WebCore::ColorChoosers now.
(WebCore::EmptyChromeClient::setSelectedColorInColorChooser): Ditto.
* page/Chrome.cpp:
(WebCore::Chrome::cleanupColorChooser): Added colorChooser argument because there are many WebCore::ColorChoosers now.
(WebCore::Chrome::setSelectedColorInColorChooser): Ditto.
* page/Chrome.h:
* page/ChromeClient.h:
* platform/ColorChooser.cpp:
(WebCore::ColorChooserClient::~ColorChooserClient):
(WebCore::ColorChooserClient::newColorChooser): Creates a new color chooser that is connected to itself.
(WebCore::ColorChooserClient::discardChooser): Discards the connected color chooser.
(WebCore::ColorChooser::ColorChooser): ColorChooser is RefCounted.
(WebCore::ColorChooser::create): Creates a ColorChooser that is connected to the given ColorChooserClient.
(WebCore::ColorChooser::~ColorChooser):
(WebCore::ColorChooser::didChooseColor): Called from WebKit side when user chose a color. Calls ColorChooserClient::didChooseColor
(WebCore::ColorChooser::didCleanup): Called from WebKit side when user color chooser was cleaned up. Calls ColorChooserClient::didCleanup
* platform/ColorChooser.h:
(WebCore::ColorChooserClient::chooser): Returns the current ColorChooser.
(WebCore::ColorChooser::disconnectClient): Disconnects the ColorChooserClient.
* testing/Internals.cpp:
(WebCore::Internals::selectColorInColorChooser): Added element argument. This calls didChooseColor on the ColorChooser of that element.
* testing/Internals.h:
* testing/Internals.idl: Removed connectColorChooserClient and updated selectColorInColorChooser.

LayoutTests:

* fast/forms/color/input-color-onchange-event.html: Changed to match the changes to window.internals object

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/forms/color/input-color-onchange-event.html
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/html/ColorInputType.cpp
Source/WebCore/html/ColorInputType.h
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/html/HTMLInputElement.h
Source/WebCore/loader/EmptyClients.h
Source/WebCore/page/Chrome.cpp
Source/WebCore/page/Chrome.h
Source/WebCore/page/ChromeClient.h
Source/WebCore/platform/ColorChooser.cpp
Source/WebCore/platform/ColorChooser.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index d5c538f..b2f9288 100644 (file)
@@ -1,3 +1,12 @@
+2011-11-07  Keishi Hattori  <keishi@webkit.org>
+
+        Change ColorChooser from singleton to ordinary object
+        https://bugs.webkit.org/show_bug.cgi?id=71644
+
+        Reviewed by Kent Tamura.
+
+        * fast/forms/color/input-color-onchange-event.html: Changed to match the changes to window.internals object
+
 2011-11-07  Alexander Pavlov  <apavlov@chromium.org>
 
         Web Inspector: Cannot edit elements commented with <!--
index 2636e83..e3a22fb 100644 (file)
@@ -7,21 +7,32 @@
 <p id="description"></p>
 <div id="console"></div>
 <script>
-description('Test if change event fires properly when color chooser changes. Bug 66848');
+description('Test if change event fires properly when color chooser changes. Bug 66848 <br> To manually test this, click on the input color element in the top left corner and change the value from the color chooser. See if the number of "value changed" messages matches the number of times you changed the color.');
 
 var input = document.createElement('input');
 input.type = 'color';
 input.value = '#000000';
+document.body.appendChild(input);
+
+input.style.position = 'absolute';
+input.style.left = '0';
+input.style.top = '0';
+input.style.width = '20px';
+input.style.height = '20px';
+
 input.onchange = function() {
     debug("value changed to " + input.value);
-    finishJSTest();
 };
-if (!internals.connectColorChooserClient(input))
-    testFailed("Could not connect ColorChooserClient.");
-internals.selectColorInColorChooser('#ff0000');
+
+eventSender.mouseMoveTo(10, 10);
+eventSender.mouseDown();
+eventSender.mouseUp();
+
 // input.onchange should be called
-internals.selectColorInColorChooser('#ff0000');
+internals.selectColorInColorChooser(input, '#ff0000');
 // input.onchange should not be called
+internals.selectColorInColorChooser(input, '#ff0000');
+
 shouldBe('input.value', '"#ff0000"');
 </script>
 </body>
index 2c2c6b2..b5d3470 100644 (file)
@@ -1,3 +1,49 @@
+2011-11-07  Keishi Hattori  <keishi@webkit.org>
+
+        Change ColorChooser from singleton to ordinary object
+        https://bugs.webkit.org/show_bug.cgi?id=71644
+
+        Reviewed by Kent Tamura.
+
+        Changing WebCore::ColorChooser from a singleton to an ordinary object can broaden how browsers implement the color chooser interface.
+
+        * WebCore.exp.in:
+        * html/ColorInputType.cpp:
+        (WebCore::ColorInputType::~ColorInputType):
+        (WebCore::ColorInputType::setValue): If a chooser exists, calls Chrome::setSelectedColorInColorChooser
+        (WebCore::ColorInputType::handleDOMActivateEvent):
+        (WebCore::ColorInputType::detach):
+        (WebCore::ColorInputType::didCleanup): Called after cleanup is complete.
+        (WebCore::ColorInputType::cleanupColorChooser): Renamed from cleanupColorChooserIfCurrentClient.
+        * html/ColorInputType.h:
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::selectColorInColorChooser):
+        * html/HTMLInputElement.h:
+        * loader/EmptyClients.h:
+        (WebCore::EmptyChromeClient::cleanupColorChooser): Added colorChooser argument because there are many WebCore::ColorChoosers now.
+        (WebCore::EmptyChromeClient::setSelectedColorInColorChooser): Ditto.
+        * page/Chrome.cpp:
+        (WebCore::Chrome::cleanupColorChooser): Added colorChooser argument because there are many WebCore::ColorChoosers now.
+        (WebCore::Chrome::setSelectedColorInColorChooser): Ditto.
+        * page/Chrome.h:
+        * page/ChromeClient.h:
+        * platform/ColorChooser.cpp:
+        (WebCore::ColorChooserClient::~ColorChooserClient):
+        (WebCore::ColorChooserClient::newColorChooser): Creates a new color chooser that is connected to itself.
+        (WebCore::ColorChooserClient::discardChooser): Discards the connected color chooser.
+        (WebCore::ColorChooser::ColorChooser): ColorChooser is RefCounted.
+        (WebCore::ColorChooser::create): Creates a ColorChooser that is connected to the given ColorChooserClient.
+        (WebCore::ColorChooser::~ColorChooser):
+        (WebCore::ColorChooser::didChooseColor): Called from WebKit side when user chose a color. Calls ColorChooserClient::didChooseColor
+        (WebCore::ColorChooser::didCleanup): Called from WebKit side when user color chooser was cleaned up. Calls ColorChooserClient::didCleanup
+        * platform/ColorChooser.h:
+        (WebCore::ColorChooserClient::chooser): Returns the current ColorChooser.
+        (WebCore::ColorChooser::disconnectClient): Disconnects the ColorChooserClient.
+        * testing/Internals.cpp:
+        (WebCore::Internals::selectColorInColorChooser): Added element argument. This calls didChooseColor on the ColorChooser of that element.
+        * testing/Internals.h:
+        * testing/Internals.idl: Removed connectColorChooserClient and updated selectColorInColorChooser.
+
 2011-11-07  Alexander Pavlov  <apavlov@chromium.org>
 
         Web Inspector: Cannot edit elements commented with <!--
index 1bbbbdb..12d0e26 100644 (file)
@@ -1979,10 +1979,8 @@ __ZN7WebCore14ResourceHandle24setDefaultStorageSessionEPK21__CFURLStorageSession
 #endif
 
 #if ENABLE(INPUT_COLOR)
-__ZN7WebCore12ColorChooser7chooserEv
-__ZNK7WebCore12ColorChooser13didChooseColorERKNS_5ColorE
 __ZN7WebCore5ColorC1ERKN3WTF6StringE
-__ZN7WebCore16HTMLInputElement21connectToColorChooserEv
+__ZN7WebCore16HTMLInputElement25selectColorInColorChooserERKNS_5ColorE
 #endif
 
 #if !defined(NSGEOMETRY_TYPES_SAME_AS_CGGEOMETRY_TYPES)
index 8e79421..57ee577 100644 (file)
@@ -66,7 +66,7 @@ PassOwnPtr<InputType> ColorInputType::create(HTMLInputElement* element)
 
 ColorInputType::~ColorInputType()
 {
-    cleanupColorChooserIfCurrentClient();
+    cleanupColorChooser();
 }
 
 bool ColorInputType::isColorControl() const
@@ -126,10 +126,9 @@ void ColorInputType::setValue(const String& value, bool valueChanged, bool sendC
         return;
 
     updateColorSwatch();
-    if (ColorChooser::chooser()->client() == this) {
-        if (Chrome* chrome = this->chrome())
-            chrome->setSelectedColorInColorChooser(valueAsColor());
-    }
+    Chrome* chrome = this->chrome();
+    if (chrome && chooser())
+        chrome->setSelectedColorInColorChooser(chooser(), valueAsColor());
 }
 
 void ColorInputType::handleDOMActivateEvent(Event* event)
@@ -140,16 +139,14 @@ void ColorInputType::handleDOMActivateEvent(Event* event)
     if (!ScriptController::processingUserGesture())
         return;
 
-    if (Chrome* chrome = this->chrome()) {
-        ColorChooser::chooser()->connectClient(this);
-        chrome->openColorChooser(ColorChooser::chooser(), valueAsColor());
-    }
+    if (Chrome* chrome = this->chrome())
+        chrome->openColorChooser(newColorChooser(), valueAsColor());
     event->setDefaultHandled();
 }
 
 void ColorInputType::detach()
 {
-    cleanupColorChooserIfCurrentClient();
+    cleanupColorChooser();
 }
 
 void ColorInputType::didChooseColor(const Color& color)
@@ -161,17 +158,17 @@ void ColorInputType::didChooseColor(const Color& color)
     element()->dispatchFormControlChangeEvent();
 }
 
-bool ColorInputType::isColorInputType() const
+void ColorInputType::didCleanup()
 {
-    return true;
+    discardChooser();
 }
 
-void ColorInputType::cleanupColorChooserIfCurrentClient() const
+void ColorInputType::cleanupColorChooser()
 {
-    if (ColorChooser::chooser()->client() != this)
-        return;
-    if (Chrome* chrome = this->chrome())
-        chrome->cleanupColorChooser();
+    Chrome* chrome = this->chrome();
+    if (chrome && chooser())
+        chrome->cleanupColorChooser(chooser());
+    discardChooser();
 }
 
 void ColorInputType::updateColorSwatch()
index ad8afc4..4b233d3 100644 (file)
@@ -57,10 +57,10 @@ private:
     virtual void detach();
 
     // ColorChooserClient implementation.
-    virtual void didChooseColor(const Color&);
-    virtual bool isColorInputType() const;
+    virtual void didChooseColor(const Color&) OVERRIDE;
+    virtual void didCleanup() OVERRIDE;
 
-    void cleanupColorChooserIfCurrentClient() const;
+    void cleanupColorChooser();
     void updateColorSwatch();
     HTMLElement* shadowColorSwatch() const;
 };
index 397e932..667118b 100644 (file)
@@ -1537,12 +1537,14 @@ bool HTMLInputElement::recalcWillValidate() const
 }
 
 #if ENABLE(INPUT_COLOR)
-bool HTMLInputElement::connectToColorChooser()
+void HTMLInputElement::selectColorInColorChooser(const Color& color)
 {
     if (!m_inputType->isColorControl())
-        return false;
-    ColorChooser::chooser()->connectClient(static_cast<ColorInputType*>(m_inputType.get()));
-    return true;
+        return;
+    RefPtr<ColorChooser> chooser = static_cast<ColorInputType*>(m_inputType.get())->chooser();
+    if (!chooser)
+        return;
+    chooser->didChooseColor(color);
 }
 #endif
     
index 496d4d6..8bad5d7 100644 (file)
@@ -233,7 +233,7 @@ public:
 
 #if ENABLE(INPUT_COLOR)
     // For test purposes.
-    bool connectToColorChooser();
+    void selectColorInColorChooser(const Color&);
 #endif
 
     String defaultToolTip() const;
index 2b75efb..13cc08f 100644 (file)
@@ -197,8 +197,8 @@ public:
 
 #if ENABLE(INPUT_COLOR)
     void openColorChooser(ColorChooser*, const Color&) { }
-    void cleanupColorChooser() { }
-    void setSelectedColorInColorChooser(const Color&) { }
+    void cleanupColorChooser(ColorChooser*) { }
+    void setSelectedColorInColorChooser(ColorChooser*, const Color&) { }
 #endif
 
     virtual void runOpenPanel(Frame*, PassRefPtr<FileChooser>) { }
index 7e3c6c3..6d0da8f 100644 (file)
@@ -463,14 +463,14 @@ void Chrome::openColorChooser(ColorChooser* colorChooser, const Color& initialCo
     m_client->openColorChooser(colorChooser, initialColor);
 }
 
-void Chrome::cleanupColorChooser()
+void Chrome::cleanupColorChooser(ColorChooser* colorChooser)
 {
-    m_client->cleanupColorChooser();
+    m_client->cleanupColorChooser(colorChooser);
 }
 
-void Chrome::setSelectedColorInColorChooser(const Color& color)
+void Chrome::setSelectedColorInColorChooser(ColorChooser* colorChooser, const Color& color)
 {
-    m_client->setSelectedColorInColorChooser(color);
+    m_client->setSelectedColorInColorChooser(colorChooser, color);
 }
 #endif
 
index 45666ea..51c5922 100644 (file)
@@ -157,8 +157,8 @@ namespace WebCore {
 
 #if ENABLE(INPUT_COLOR)
         void openColorChooser(ColorChooser*, const Color&);
-        void cleanupColorChooser();
-        void setSelectedColorInColorChooser(const Color&);
+        void cleanupColorChooser(ColorChooser*);
+        void setSelectedColorInColorChooser(ColorChooser*, const Color&);
 #endif
 
         void runOpenPanel(Frame*, PassRefPtr<FileChooser>);
index cde7c42..7ccb77a 100644 (file)
@@ -228,8 +228,8 @@ namespace WebCore {
 
 #if ENABLE(INPUT_COLOR)
         virtual void openColorChooser(ColorChooser*, const Color&) = 0;
-        virtual void cleanupColorChooser() = 0;
-        virtual void setSelectedColorInColorChooser(const Color&) = 0;
+        virtual void cleanupColorChooser(ColorChooser*) = 0;
+        virtual void setSelectedColorInColorChooser(ColorChooser*, const Color&) = 0;
 #endif
 
         virtual void runOpenPanel(Frame*, PassRefPtr<FileChooser>) = 0;
index 6e0e7cb..173f3f8 100644 (file)
@@ -38,36 +38,50 @@ namespace WebCore {
 
 ColorChooserClient::~ColorChooserClient()
 {
-    ColorChooser::chooser()->disconnectClient(this);
+    discardChooser();
 }
 
-static ColorChooser* staticChooser = 0;
+ColorChooser* ColorChooserClient::newColorChooser()
+{
+    discardChooser();
+
+    m_chooser = ColorChooser::create(this);
+    return m_chooser.get();
+}
+
+void ColorChooserClient::discardChooser()
+{
+    if (m_chooser)
+        m_chooser->disconnectClient();
+    m_chooser.clear();
+}
 
-ColorChooser* ColorChooser::chooser()
+inline ColorChooser::ColorChooser(ColorChooserClient* client)
+    : m_client(client)
 {
-    if (!staticChooser)
-        staticChooser = adoptPtr(new ColorChooser()).leakPtr();
-    return staticChooser;
 }
 
-void ColorChooser::connectClient(ColorChooserClient* client)
+PassRefPtr<ColorChooser> ColorChooser::create(ColorChooserClient* client)
 {
-    if (client != m_client)
-        m_client = client;
+    return adoptRef(new ColorChooser(client));
 }
 
-void ColorChooser::disconnectClient(ColorChooserClient* client)
+ColorChooser::~ColorChooser()
 {
-    if (client == m_client)
-        m_client = 0;
 }
 
-void ColorChooser::didChooseColor(const Color& color) const
+void ColorChooser::didChooseColor(const Color& color)
 {
     if (m_client)
         m_client->didChooseColor(color);
 }
 
+void ColorChooser::didCleanup()
+{
+    if (m_client)
+        m_client->didCleanup();
+}
+
 }
 
 #endif // ENABLE(INPUT_COLOR)
index 1e68788..ba11134 100644 (file)
@@ -31,6 +31,8 @@
 #define ColorChooser_h
 
 #include "Color.h"
+#include <wtf/RefCounted.h>
+#include <wtf/RefPtr.h>
 
 #if ENABLE(INPUT_COLOR)
 
@@ -42,23 +44,29 @@ class ColorChooserClient {
 public:
     virtual ~ColorChooserClient();
     virtual void didChooseColor(const Color&) = 0;
-    virtual bool isColorInputType() const { return false; }
+    virtual void didCleanup() = 0;
+    ColorChooser* chooser() { return m_chooser.get(); }
+
+protected:
+    ColorChooser* newColorChooser();
+    void discardChooser();
+
+private:
+    RefPtr<ColorChooser> m_chooser;
 };
 
-class ColorChooser {
+class ColorChooser : public RefCounted<ColorChooser> {
 public:
-    static ColorChooser* chooser();
+    static PassRefPtr<ColorChooser> create(ColorChooserClient*);
+    ~ColorChooser();
 
-    ColorChooserClient* client() const { return m_client; };
-    void connectClient(ColorChooserClient*);
-    void disconnectClient(ColorChooserClient*);
+    void disconnectClient() { m_client = 0; }
 
-    void didChooseColor(const Color&) const;
+    void didChooseColor(const Color&);
+    void didCleanup();
 
 private:
-    ColorChooser()
-        : m_client(0)
-    { }
+    ColorChooser(ColorChooserClient*);
 
     ColorChooserClient* m_client;
 };
index aefa4d9..b1ddd19 100644 (file)
@@ -174,19 +174,14 @@ String Internals::shadowPseudoId(Element* element, ExceptionCode& ec)
 }
 
 #if ENABLE(INPUT_COLOR)
-bool Internals::connectColorChooserClient(Element* element)
+void Internals::selectColorInColorChooser(Element* element, const String& colorValue)
 {
     if (!element->hasTagName(HTMLNames::inputTag))
-        return false;
+        return;
     HTMLInputElement* inputElement = element->toInputElement();
     if (!inputElement)
-        return false;
-    return inputElement->connectToColorChooser();
-}
-
-void Internals::selectColorInColorChooser(const String& colorValue)
-{
-    ColorChooser::chooser()->colorSelected(Color(colorValue));
+        return;
+    inputElement->selectColorInColorChooser(Color(colorValue));
 }
 #endif
 
index 18b4225..ed0f6d9 100644 (file)
@@ -60,8 +60,7 @@ public:
     Element* getElementByIdInShadowRoot(Node* shadowRoot, const String& id, ExceptionCode&);
 
 #if ENABLE(INPUT_COLOR)
-    bool connectColorChooserClient(Element*);
-    void selectColorInColorChooser(const String& colorValue);
+    void selectColorInColorChooser(Element*, const String& colorValue);
 #endif
 
 #if ENABLE(INSPECTOR)
index 3850bbb..4a36638 100644 (file)
@@ -39,8 +39,7 @@ module window {
         Element getElementByIdInShadowRoot(in Node shadowRoot, in DOMString id) raises(DOMException);
 
 #if defined(ENABLE_INPUT_COLOR) && ENABLE_INPUT_COLOR
-        boolean connectColorChooserClient(in Element element);
-        void selectColorInColorChooser(in DOMString colorValue);
+        void selectColorInColorChooser(in Element element, in DOMString colorValue);
 #endif
 
         void setInspectorResourcesDataSizeLimits(in Document document, in long maximumResourcesContentSize, in long maximumSingleResourceContentSize) raises(DOMException);