Diff
Modified: trunk/LayoutTests/ChangeLog (101730 => 101731)
--- trunk/LayoutTests/ChangeLog 2011-12-02 01:58:49 UTC (rev 101730)
+++ trunk/LayoutTests/ChangeLog 2011-12-02 02:15:48 UTC (rev 101731)
@@ -1,3 +1,17 @@
+2011-12-01 Shinya Kawanaka <[email protected]>
+
+ Asynchronous SpellChecker should consider multiple requests.
+ https://bugs.webkit.org/show_bug.cgi?id=72939
+
+ Reviewed by Hajime Morita.
+
+ 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-01 Takashi Toyoshima <[email protected]>
bufferedAmount calculation is wrong in CLOSING and CLOSED state.
Added: trunk/LayoutTests/editing/spelling/spellcheck-queue-expected.txt (0 => 101731)
--- trunk/LayoutTests/editing/spelling/spellcheck-queue-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/spelling/spellcheck-queue-expected.txt 2011-12-02 02:15:48 UTC (rev 101731)
@@ -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'
+
Added: trunk/LayoutTests/editing/spelling/spellcheck-queue.html (0 => 101731)
--- trunk/LayoutTests/editing/spelling/spellcheck-queue.html (rev 0)
+++ trunk/LayoutTests/editing/spelling/spellcheck-queue.html 2011-12-02 02:15:48 UTC (rev 101731)
@@ -0,0 +1,188 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+<script src=""
+</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=""
+</body>
+</html>
Modified: trunk/LayoutTests/platform/gtk/Skipped (101730 => 101731)
--- trunk/LayoutTests/platform/gtk/Skipped 2011-12-02 01:58:49 UTC (rev 101730)
+++ trunk/LayoutTests/platform/gtk/Skipped 2011-12-02 02:15:48 UTC (rev 101731)
@@ -1216,6 +1216,7 @@
# 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.
Modified: trunk/LayoutTests/platform/qt/Skipped (101730 => 101731)
--- trunk/LayoutTests/platform/qt/Skipped 2011-12-02 01:58:49 UTC (rev 101730)
+++ trunk/LayoutTests/platform/qt/Skipped 2011-12-02 02:15:48 UTC (rev 101731)
@@ -1001,6 +1001,7 @@
# 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
Modified: trunk/Source/WebCore/ChangeLog (101730 => 101731)
--- trunk/Source/WebCore/ChangeLog 2011-12-02 01:58:49 UTC (rev 101730)
+++ trunk/Source/WebCore/ChangeLog 2011-12-02 02:15:48 UTC (rev 101731)
@@ -1,3 +1,37 @@
+2011-12-01 Shinya Kawanaka <[email protected]>
+
+ Asynchronous SpellChecker should consider multiple requests.
+ https://bugs.webkit.org/show_bug.cgi?id=72939
+
+ Reviewed by Hajime Morita.
+
+ 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-01 Takashi Toyoshima <[email protected]>
bufferedAmount calculation is wrong in CLOSING and CLOSED state.
Modified: trunk/Source/WebCore/editing/SpellChecker.cpp (101730 => 101731)
--- trunk/Source/WebCore/editing/SpellChecker.cpp 2011-12-02 01:58:49 UTC (rev 101730)
+++ trunk/Source/WebCore/editing/SpellChecker.cpp 2011-12-02 02:15:48 UTC (rev 101731)
@@ -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 @@
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,19 +116,9 @@
bool SpellChecker::canCheckAsynchronously(Range* range) const
{
- return client() && isCheckable(range) && isAsynchronousEnabled() && !isBusy();
+ return client() && isCheckable(range) && isAsynchronousEnabled();
}
-bool SpellChecker::isBusy() const
-{
- return m_requestRange.get();
-}
-
-bool SpellChecker::isValid(int sequence) const
-{
- return m_requestRange.get() && m_requestText.length() && m_requestSequence == sequence;
-}
-
bool SpellChecker::isCheckable(Range* range) const
{
return range && range->firstNode() && range->firstNode()->renderer();
@@ -114,16 +129,39 @@
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);
- if (!initRequest(range))
+ client()->requestCheckingOfString(this, request->sequence(), request->mask(), request->text());
+ m_processingRequest = request;
+}
+
+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 @@
// 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 @@
// 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);
}
Modified: trunk/Source/WebCore/editing/SpellChecker.h (101730 => 101731)
--- trunk/Source/WebCore/editing/SpellChecker.h 2011-12-02 01:58:49 UTC (rev 101730)
+++ trunk/Source/WebCore/editing/SpellChecker.h 2011-12-02 02:15:48 UTC (rev 101731)
@@ -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 @@
~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;
- RefPtr<Range> m_requestRange;
- String m_requestText;
- int m_requestSequence;
+ Timer<SpellChecker> m_timerToProcessQueuedRequest;
+
+ RefPtr<SpellCheckRequest> m_processingRequest;
+ RequestQueue m_requestQueue;
};
} // namespace WebCore
