Attachment 117819 Details for Bug 72939 – Patch

[patch] Patch

bug-72939-20111205112132.patch (text/plain), 18.46 KB, created by Shinya Kawanaka on 2011-12-04 18:21:34 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Shinya Kawanaka

Created: 2011-12-04 18:21:34 PST

Size: 18.46 KB

patch

obsolete

>Subversion Revision: 101969
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a69b1b99f010fca297f7eaa285fe934098b39ac6..b7359c983bed33910406c8b228834a20a4d403a9 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,37 @@
>+2011-12-04  Shinya Kawanaka  <shinyak@google.com>
>+
>+        Asynchronous SpellChecker should consider multiple requests.
>+        https://bugs.webkit.org/show_bug.cgi?id=72939
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Now SpellChecker saves a request when it is processing the previous spellcheck request.
>+        If there is a request having the same root editable element, the older request is replaced by newer request
>+
>+        Test: editing/spelling/spellcheck-queue.html
>+
>+        * editing/SpellChecker.cpp:
>+        (WebCore::SpellChecker::SpellCheckRequest::SpellCheckRequest):
>+          A structure to have spell check request.
>+        (WebCore::SpellChecker::SpellCheckRequest::sequence):
>+        (WebCore::SpellChecker::SpellCheckRequest::range):
>+        (WebCore::SpellChecker::SpellCheckRequest::text):
>+        (WebCore::SpellChecker::SpellCheckRequest::mask):
>+        (WebCore::SpellChecker::SpellCheckRequest::rootEditableElement):
>+        (WebCore::SpellChecker::SpellChecker):
>+        (WebCore::SpellChecker::createRequest):
>+        (WebCore::SpellChecker::timerFiredToProcessQueuedRequest):
>+          When timer is fired, queued request is processed if any.
>+        (WebCore::SpellChecker::canCheckAsynchronously):
>+        (WebCore::SpellChecker::requestCheckingFor):
>+          When the spellchecker is processing another request, the latest request is queued.
>+        (WebCore::SpellChecker::invokeRequest):
>+        (WebCore::SpellChecker::enqueueRequest):
>+          Enqueues a request. If there is an older request whose root editable element is the same as the request,
>+          it will be replaced.
>+        (WebCore::SpellChecker::didCheck):
>+        * editing/SpellChecker.h:
>+
> 2011-12-04  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         HIERARCHY_REQUEST_ERR check in checkAcceptChild should be optimized for newChild without children
>diff --git a/Source/WebCore/editing/SpellChecker.cpp b/Source/WebCore/editing/SpellChecker.cpp
>index 1bc975af17165cba19420e8ca61f1d0b201d16a5..8ff5742174226b4f22079ba31079534e1b6a14f4 100644
>--- a/Source/WebCore/editing/SpellChecker.cpp
>+++ b/Source/WebCore/editing/SpellChecker.cpp
>@@ -45,9 +45,35 @@
> 
> namespace WebCore {
> 
>+class SpellChecker::SpellCheckRequest : public RefCounted<SpellChecker::SpellCheckRequest> {
>+public:
>+    SpellCheckRequest(int sequence, PassRefPtr<Range> range, const String& text, TextCheckingTypeMask mask)
>+        : m_sequence(sequence)
>+        , m_range(range)
>+        , m_text(text)
>+        , m_mask(mask)
>+        , m_rootEditableElement(m_range->startContainer()->rootEditableElement())
>+    {
>+    }
>+
>+    int sequence() const { return m_sequence; }
>+    Range* range() const { return m_range.get(); }
>+    const String& text() const { return m_text; }
>+    TextCheckingTypeMask mask() const { return m_mask; }
>+    Element* rootEditableElement() const { return m_rootEditableElement; }
>+
>+private:
>+    int m_sequence;
>+    RefPtr<Range> m_range;
>+    String m_text;
>+    TextCheckingTypeMask m_mask;
>+    Element* m_rootEditableElement;
>+};
>+
> SpellChecker::SpellChecker(Frame* frame)
>     : m_frame(frame)
>-    , m_requestSequence(0)
>+    , m_lastRequestedSequence(0)
>+    , m_timerToProcessQueuedRequest(this, &SpellChecker::timerFiredToProcessQueuedRequest)
> {
> }
> 
>@@ -63,25 +89,24 @@ TextCheckerClient* SpellChecker::client() const
>     return page->editorClient()->textChecker();
> }
> 
>-bool SpellChecker::initRequest(PassRefPtr<Range> range)
>+PassRefPtr<SpellChecker::SpellCheckRequest> SpellChecker::createRequest(TextCheckingTypeMask mask, PassRefPtr<Range> range)
> {
>     ASSERT(canCheckAsynchronously(range.get()));
> 
>     String text = range->text();
>     if (!text.length())
>-        return false;
>+        return PassRefPtr<SpellCheckRequest>();
> 
>-    m_requestRange = range;
>-    m_requestText = text;
>-    m_requestSequence++;
>-
>-    return true;
>+    return adoptRef(new SpellCheckRequest(++m_lastRequestedSequence, range, text, mask));
> }
> 
>-void SpellChecker::clearRequest()
>+void SpellChecker::timerFiredToProcessQueuedRequest(Timer<SpellChecker>*)
> {
>-    m_requestRange.clear();
>-    m_requestText = String();
>+    ASSERT(!m_requestQueue.isEmpty());
>+    if (m_requestQueue.isEmpty())
>+        return;
>+
>+    invokeRequest(m_requestQueue.takeFirst());
> }
> 
> bool SpellChecker::isAsynchronousEnabled() const
>@@ -91,17 +116,7 @@ bool SpellChecker::isAsynchronousEnabled() const
> 
> bool SpellChecker::canCheckAsynchronously(Range* range) const
> {
>-    return client() && isCheckable(range) && isAsynchronousEnabled() && !isBusy();
>-}
>-
>-bool SpellChecker::isBusy() const
>-{
>-    return m_requestRange.get();
>-}
>-
>-bool SpellChecker::isValid(int sequence) const
>-{
>-    return m_requestRange.get() && m_requestText.length() && m_requestSequence == sequence;
>+    return client() && isCheckable(range) && isAsynchronousEnabled();
> }
> 
> bool SpellChecker::isCheckable(Range* range) const
>@@ -114,16 +129,39 @@ void SpellChecker::requestCheckingFor(TextCheckingTypeMask mask, PassRefPtr<Rang
>     if (!canCheckAsynchronously(range.get()))
>         return;
> 
>-    doRequestCheckingFor(mask, range);
>+    RefPtr<SpellCheckRequest> request(createRequest(mask, range));
>+    if (!request)
>+        return;
>+
>+    if (m_timerToProcessQueuedRequest.isActive() || m_processingRequest) {
>+        enqueueRequest(request.release());
>+        return;
>+    }
>+
>+    invokeRequest(request.release());
> }
> 
>-void SpellChecker::doRequestCheckingFor(TextCheckingTypeMask mask, PassRefPtr<Range> range)
>+void SpellChecker::invokeRequest(PassRefPtr<SpellCheckRequest> request)
> {
>-    ASSERT(canCheckAsynchronously(range.get()));
>+    ASSERT(!m_processingRequest);
>+
>+    m_processingRequest = request;
>+    client()->requestCheckingOfString(this, m_processingRequest->sequence(), m_processingRequest->mask(), m_processingRequest->text());
>+}
> 
>-    if (!initRequest(range))
>+void SpellChecker::enqueueRequest(PassRefPtr<SpellCheckRequest> request)
>+{
>+    ASSERT(request);
>+
>+    for (RequestQueue::iterator it = m_requestQueue.begin(); it != m_requestQueue.end(); ++it) {
>+        if (request->rootEditableElement() != (*it)->rootEditableElement())
>+            continue;
>+
>+        *it = request;
>         return;
>-    client()->requestCheckingOfString(this, m_requestSequence, mask, m_requestText);
>+    }
>+
>+    m_requestQueue.append(request);
> }
> 
> static bool forwardIterator(PositionIterator& iterator, int distance)
>@@ -159,16 +197,16 @@ static DocumentMarker::MarkerType toMarkerType(TextCheckingType type)
> // Currenntly ignoring TextCheckingResult::details but should be handled. See Bug 56368.
> void SpellChecker::didCheck(int sequence, const Vector<TextCheckingResult>& results)
> {
>-    if (!isValid(sequence))
>-        return;
>+    ASSERT(m_processingRequest);
> 
>-    if (!isCheckable(m_requestRange.get())) {
>-        clearRequest();
>+    ASSERT(m_processingRequest->sequence() == sequence);
>+    if (m_processingRequest->sequence() != sequence) {
>+        m_requestQueue.clear();
>         return;
>     }
> 
>     int startOffset = 0;
>-    PositionIterator start = m_requestRange->startPosition();
>+    PositionIterator start = m_processingRequest->range()->startPosition();
>     for (size_t i = 0; i < results.size(); ++i) {
>         if (results[i].type != TextCheckingTypeSpelling && results[i].type != TextCheckingTypeGrammar)
>             continue;
>@@ -186,17 +224,19 @@ void SpellChecker::didCheck(int sequence, const Vector<TextCheckingResult>& resu
>         // spellings in the background. To avoid adding markers to the words modified by users or
>         // JavaScript applications, retrieve the words in the specified region and compare them with
>         // the original ones.
>-        RefPtr<Range> range = Range::create(m_requestRange->ownerDocument(), start, end);
>+        RefPtr<Range> range = Range::create(m_processingRequest->range()->ownerDocument(), start, end);
>         // FIXME: Use textContent() compatible string conversion.
>         String destination = range->text();
>-        String source = m_requestText.substring(results[i].location, results[i].length);
>+        String source = m_processingRequest->text().substring(results[i].location, results[i].length);
>         if (destination == source)
>-            m_requestRange->ownerDocument()->markers()->addMarker(range.get(), toMarkerType(results[i].type));
>+            m_processingRequest->range()->ownerDocument()->markers()->addMarker(range.get(), toMarkerType(results[i].type));
> 
>         startOffset = results[i].location;
>     }
> 
>-    clearRequest();
>+    m_processingRequest.clear();
>+    if (!m_requestQueue.isEmpty())
>+        m_timerToProcessQueuedRequest.startOneShot(0);
> }
> 
> 
>diff --git a/Source/WebCore/editing/SpellChecker.h b/Source/WebCore/editing/SpellChecker.h
>index 3a4148993aa7a390d34342987c2ab088387c8a91..702f369d5657aebc459b5113f6e216a5dc28f1ff 100644
>--- a/Source/WebCore/editing/SpellChecker.h
>+++ b/Source/WebCore/editing/SpellChecker.h
>@@ -28,6 +28,8 @@
> 
> #include "PlatformString.h"
> #include "TextChecking.h"
>+#include "Timer.h"
>+#include <wtf/Deque.h>
> #include <wtf/RefPtr.h>
> #include <wtf/Noncopyable.h>
> #include <wtf/Vector.h>
>@@ -47,24 +49,28 @@ public:
>     ~SpellChecker();
> 
>     bool isAsynchronousEnabled() const;
>-    bool canCheckAsynchronously(Range*) const;
>-    bool isBusy() const;
>-    bool isValid(int sequence) const;
>     bool isCheckable(Range*) const;
>     void requestCheckingFor(TextCheckingTypeMask, PassRefPtr<Range>);
>     void didCheck(int sequence, const Vector<TextCheckingResult>&);
> 
> private:
>-    bool initRequest(PassRefPtr<Range>);
>-    void clearRequest();
>-    void doRequestCheckingFor(TextCheckingTypeMask, PassRefPtr<Range>);
>+    class SpellCheckRequest;
>+    typedef Deque<RefPtr<SpellCheckRequest> > RequestQueue;
>+
>+    bool canCheckAsynchronously(Range*) const;
>+    PassRefPtr<SpellCheckRequest> createRequest(TextCheckingTypeMask, PassRefPtr<Range>);
>     TextCheckerClient* client() const;
>+    void timerFiredToProcessQueuedRequest(Timer<SpellChecker>*);
>+    void invokeRequest(PassRefPtr<SpellCheckRequest>);
>+    void enqueueRequest(PassRefPtr<SpellCheckRequest>);
> 
>     Frame* m_frame;
>+    int m_lastRequestedSequence;
>+
>+    Timer<SpellChecker> m_timerToProcessQueuedRequest;
> 
>-    RefPtr<Range> m_requestRange;
>-    String m_requestText;
>-    int m_requestSequence;
>+    RefPtr<SpellCheckRequest> m_processingRequest;
>+    RequestQueue m_requestQueue;
> };
> 
> } // namespace WebCore
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 50bfdab604c7160dac44f8fa8694369a9413a43b..37cb5b5add23dfd93d885ff5048b7d5fe47c455c 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2011-12-04  Shinya Kawanaka  <shinyak@google.com>
>+
>+        Asynchronous SpellChecker should consider multiple requests.
>+        https://bugs.webkit.org/show_bug.cgi?id=72939
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Tests for multiple spellcheck requests.
>+
>+        * editing/spelling/spellcheck-queue-expected.txt: Added.
>+        * editing/spelling/spellcheck-queue.html: Added.
>+        * platform/gtk/Skipped:
>+        * platform/qt/Skipped:
>+
> 2011-12-04  Darin Adler  <darin@apple.com>
> 
>         Removed obsolete expected results that should have been removed when converting to a ref test.
>diff --git a/LayoutTests/editing/spelling/spellcheck-queue-expected.txt b/LayoutTests/editing/spelling/spellcheck-queue-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..588c55cc824221726c38225607263071e264b6db
>--- /dev/null
>+++ b/LayoutTests/editing/spelling/spellcheck-queue-expected.txt
>@@ -0,0 +1,18 @@
>+For Bug 72939: Asynchronous SpellChecker should consider multiple requests.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+PASS INPUT has a marker on 'zz apple orange'
>+PASS TEXTAREA has a marker on 'zz apple orange'
>+PASS DIV has a marker on 'zz apple orange'
>+PASS INPUT has a marker on 'zz apple orange'
>+PASS TEXTAREA has a marker on 'zz apple orange'
>+PASS DIV has a marker on 'zz apple orange'
>+PASS INPUT has a marker on 'zz apple orange'
>+PASS TEXTAREA has a marker on 'zz apple orange'
>+PASS DIV has a marker on 'zz apple orange'
>+
>diff --git a/LayoutTests/editing/spelling/spellcheck-queue.html b/LayoutTests/editing/spelling/spellcheck-queue.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..1a7e89ad6bbce74e683fbcc0cc46928e963eb278
>--- /dev/null
>+++ b/LayoutTests/editing/spelling/spellcheck-queue.html
>@@ -0,0 +1,188 @@
>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
>+<html>
>+<head>
>+<script src="../../fast/js/resources/js-test-pre.js"></script>
>+<script src="resources/js-test-selection-shared.js"></script>
>+</head>
>+<body>
>+<p id="description"></p>
>+<div id="console"></div>
>+<script>
>+description('For Bug 72939: Asynchronous SpellChecker should consider multiple requests.');
>+
>+if (window.layoutTestController) {
>+    layoutTestController.waitUntilDone();
>+    layoutTestController.setAsynchronousSpellCheckingEnabled(true);
>+}
>+
>+var testRoot = document.createElement("div");
>+document.body.insertBefore(testRoot, document.body.firstChild);
>+
>+var source1 = document.createElement("div");
>+source1.innerHTML = "foo bar";
>+testRoot.appendChild(source1);
>+
>+var source2 = document.createElement("div");
>+source2.innerHTML = "zz apple orange";
>+testRoot.appendChild(source2);
>+
>+function createInput(testRoot) {
>+    var e = document.createElement('input');
>+    e.setAttribute("type", "text");
>+    testRoot.appendChild(e);
>+
>+    return e;
>+}
>+
>+function createTextArea(testRoot) {
>+    var e = document.createElement("textarea");
>+    testRoot.appendChild(e);
>+
>+    return e;
>+}
>+
>+function createContentEditable(testRoot) {
>+    var e = document.createElement("div");
>+    e.setAttribute("contentEditable", "true");
>+    testRoot.appendChild(e);
>+
>+    return e;
>+}
>+
>+var destinations = [
>+    createInput(testRoot),
>+    createTextArea(testRoot),
>+    createContentEditable(testRoot),
>+    createInput(testRoot),
>+    createTextArea(testRoot),
>+    createContentEditable(testRoot),
>+    createInput(testRoot),
>+    createTextArea(testRoot),
>+    createContentEditable(testRoot),
>+];
>+
>+var sel = window.getSelection();
>+
>+var tests = [];
>+for (var i = 0; i < destinations.length; ++i) {
>+    var t = function(i) {
>+        return function() { verify(source2, destinations[i], ["zz"]); }
>+    }(i);
>+    tests.push(t);
>+}
>+
>+function verifyIfAny()
>+{
>+    var next = tests.shift();
>+    if (next) {
>+        next();
>+        return;
>+    }
>+
>+    testRoot.style.display = "none";
>+    if (window.layoutTestController) {
>+        layoutTestController.setAsynchronousSpellCheckingEnabled(false);
>+        layoutTestController.notifyDone();
>+    }
>+}
>+
>+function findFirstTextNode(node)
>+{
>+    function iterToFindFirstTextNode(node)
>+    {
>+        if (node instanceof Text)
>+            return node;
>+
>+        var childNodes = node.childNodes;
>+        for (var i = 0; i < childNodes.length; ++i) {
>+            var n = iterToFindFirstTextNode(childNodes[i]);
>+            if (n)
>+                return n;
>+        }
>+
>+        return null;
>+    }
>+
>+
>+    if (node instanceof HTMLInputElement || node instanceof HTMLTextAreaElement)
>+        return iterToFindFirstTextNode(internals.shadowRoot(node));
>+    else
>+        return iterToFindFirstTextNode(node);
>+}
>+
>+function verifyMarker(node, expectedMarked)
>+{
>+    if (!window.layoutTestController || !window.internals)
>+        return false;
>+
>+    var textNode = findFirstTextNode(node);
>+
>+    var num = internals.markerCountForNode(textNode, "spelling");
>+    if (num != expectedMarked.length)
>+        return false;
>+    for (var i = 0; i < num; ++i) {
>+        var range = internals.markerRangeForNode(textNode, "spelling", i);
>+        if (range.toString() != expectedMarked[i])
>+            return false;
>+    }
>+
>+    return true;
>+}
>+
>+function copyAndPaste(source, dest)
>+{
>+    sel.selectAllChildren(source);
>+    document.execCommand("Copy");
>+
>+    if (dest instanceof HTMLInputElement || dest instanceof HTMLTextAreaElement) {
>+        dest.value = "";
>+        dest.focus();
>+    } else {
>+        dest.innerHTML = "";
>+        sel.selectAllChildren(dest);
>+    }
>+    document.execCommand("Paste");
>+}
>+
>+function verify(source, dest, expectedMarked)
>+{
>+    var nretry = 10;
>+    var nsleep = 1;
>+    function trial() {
>+        var verified = verifyMarker(dest, expectedMarked);
>+        if (verified) {
>+            testPassed(dest.tagName + " has a marker on '" + source.innerHTML + "'");
>+            verifyIfAny();
>+            return;
>+        }
>+
>+        nretry--;
>+        if (0 == nretry) {
>+            testFailed(dest.tagName + " should have a marker on for '" + source.innerHTML + "'");
>+            verifyIfAny();
>+            return;
>+        }
>+
>+        nsleep *= 2;
>+        window.setTimeout(trial, nsleep);
>+    };
>+    trial();
>+}
>+
>+
>+// paste "foo bar"
>+for (var i = 0; i < destinations.length; ++i)
>+    copyAndPaste(source1, destinations[i]);
>+
>+// paste "zz apple orange"
>+for (var i = 0; i < destinations.length; ++i)
>+    copyAndPaste(source2, destinations[i]);
>+
>+verifyIfAny();
>+
>+var successfullyParsed = true;
>+
>+</script>
>+<script src="../../fast/js/resources/js-test-post.js"></script>
>+</body>
>+</html>
>diff --git a/LayoutTests/platform/gtk/Skipped b/LayoutTests/platform/gtk/Skipped
>index 641b9fa87133b47c3cb37dc0362c5c19cbe6e47b..86c0491e4dcc2785892ce6d0cf10b902d140e142 100644
>--- a/LayoutTests/platform/gtk/Skipped
>+++ b/LayoutTests/platform/gtk/Skipped
>@@ -1215,6 +1215,7 @@ media/event-attributes.html
> # https://bugs.webkit.org/show_bug.cgi?id=50740
> editing/spelling/spelling-backspace-between-lines.html
> editing/spelling/spellcheck-paste.html
>+editing/spelling/spellcheck-queue.html
> 
> # For https://bugs.webkit.org/show_bug.cgi?id=50758
> # These require DRT setSerializeHTTPLoads implementation to be reliable.
>diff --git a/LayoutTests/platform/qt/Skipped b/LayoutTests/platform/qt/Skipped
>index a14649a88eb606a0661baf54aeb6b88082198d6c..bd974013b20b2c7732201814836f5cd0ea42dbc2 100644
>--- a/LayoutTests/platform/qt/Skipped
>+++ b/LayoutTests/platform/qt/Skipped
>@@ -1005,6 +1005,7 @@ editing/spelling/spelling-attribute-at-child.html
> 
> # EditorClient::requestCheckingOfString() is not implemented
> editing/spelling/spellcheck-paste.html
>+editing/spelling/spellcheck-queue.html
> 
> # [Qt][GTK] editing/spelling/spellcheck-async.html fails
> # https://bugs.webkit.org/show_bug.cgi?id=73003

Actions: View | Formatted Diff | Diff

Attachments on bug 72939: 117160 | 117330 | 117377 | 117385 | 117819