[BlackBerry] Range boundaries should use endOfBlock instead of endOfLine.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Aug 2012 15:46:22 +0000 (15:46 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Aug 2012 15:46:22 +0000 (15:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=95135

The original implementation used nextLinePosition to iterate
through the field from the start of each line, and was bounded in
comparison to the endOfLine. This works fine as long as there aren't any
empty lines between paragraphs of text, since these will have
startOfLine == endOfLine and break out.

Also, protect map access with a mutex in case we get a response
before updating the map. Further, we should check the Range pointer
before using it, since its not guaranteed to be valid.

Internally reviewed by Mike Fenton.

Patch by Nima Ghanavatian <nghanavatian@rim.com> on 2012-08-28
Reviewed by Antonio Gomes.

* WebKitSupport/InputHandler.cpp:
(BlackBerry::WebKit::InputHandler::spellCheckBlock):

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

Source/WebKit/blackberry/ChangeLog
Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp
Source/WebKit/blackberry/WebKitSupport/InputHandler.h

index 5cb7bf1..a5a0bad 100644 (file)
@@ -1,3 +1,25 @@
+2012-08-28  Nima Ghanavatian  <nghanavatian@rim.com>
+
+        [BlackBerry] Range boundaries should use endOfBlock instead of endOfLine.
+        https://bugs.webkit.org/show_bug.cgi?id=95135
+
+        The original implementation used nextLinePosition to iterate
+        through the field from the start of each line, and was bounded in
+        comparison to the endOfLine. This works fine as long as there aren't any
+        empty lines between paragraphs of text, since these will have
+        startOfLine == endOfLine and break out.
+
+        Also, protect map access with a mutex in case we get a response
+        before updating the map. Further, we should check the Range pointer
+        before using it, since its not guaranteed to be valid.
+
+        Internally reviewed by Mike Fenton.
+
+        Reviewed by Antonio Gomes.
+
+        * WebKitSupport/InputHandler.cpp:
+        (BlackBerry::WebKit::InputHandler::spellCheckBlock):
+
 2012-08-28  Andrew Lo  <anlo@rim.com>
 
         [BlackBerry] One shot drawing synchronization broken
index 77f8ada..3e7548b 100644 (file)
@@ -135,6 +135,7 @@ InputHandler::InputHandler(WebPagePrivate* page)
     , m_pendingKeyboardVisibilityChange(NoChange)
     , m_delayKeyboardVisibilityChange(false)
 {
+    pthread_mutex_init(&m_sequenceMapMutex, 0);
 }
 
 InputHandler::~InputHandler()
@@ -581,6 +582,7 @@ void InputHandler::requestCheckingOfString(PassRefPtr<WebCore::TextCheckingReque
         return;
     }
 
+    BlackBerry::Platform::MutexLocker lock(&m_sequenceMapMutex);
     int32_t transactionId = m_webPage->m_client->checkSpellingOfStringAsync(checkingString, paragraphLength);
     free(checkingString);
 
@@ -598,6 +600,7 @@ void InputHandler::requestCheckingOfString(PassRefPtr<WebCore::TextCheckingReque
 
 int32_t InputHandler::convertTransactionIdToSequenceId(int32_t transactionId)
 {
+    BlackBerry::Platform::MutexLocker lock(&m_sequenceMapMutex);
     std::map<int32_t, int32_t>::iterator it = m_sequenceMap.find(transactionId);
 
     if (it == m_sequenceMap.end())
@@ -660,6 +663,7 @@ void InputHandler::spellCheckingRequestProcessed(int32_t transactionId, spannabl
 
 void InputHandler::cancelAllSpellCheckingRequests()
 {
+    BlackBerry::Platform::MutexLocker lock(&m_sequenceMapMutex);
     for (std::map<int32_t, int32_t>::iterator it = m_sequenceMap.begin(); it != m_sequenceMap.end(); ++it)
         spellCheckingRequestCancelled(it->second, true /* isSequenceId */);
     m_sequenceMap.clear();
@@ -672,7 +676,7 @@ void InputHandler::spellCheckingRequestCancelled(int32_t id, bool isSequenceId)
 
     int32_t sequenceId = isSequenceId ? id : convertTransactionIdToSequenceId(id);
     SpellChecker* spellChecker = getSpellChecker();
-    if (!spellChecker) {
+    if (!spellChecker || !sequenceId) {
         SpellingLog(LogLevelWarn, "InputHandler::spellCheckingRequestCancelled failed to cancel the request with sequenceId %d", sequenceId);
         return;
     }
@@ -856,14 +860,16 @@ void InputHandler::spellCheckBlock(VisibleSelection& visibleSelection, TextCheck
     if (!isActiveTextEdit())
         return;
 
+    RefPtr<Range> rangeForSpellChecking = visibleSelection.toNormalizedRange();
+    if (!rangeForSpellChecking || !rangeForSpellChecking->text() || !rangeForSpellChecking->text().length())
+        return;
+
     SpellChecker* spellChecker = getSpellChecker();
     if (!spellChecker) {
         SpellingLog(LogLevelInfo, "InputHandler::spellCheckBlock Failed to spellcheck the current focused element.");
         return;
     }
 
-    RefPtr<Range> rangeForSpellChecking = visibleSelection.toNormalizedRange();
-
     // If we have a batch request, try to send off the entire block.
     if (textCheckingProcessType == TextCheckingProcessBatch) {
         // If total block text is under the limited amount, send the entire chunk.
@@ -876,13 +882,16 @@ void InputHandler::spellCheckBlock(VisibleSelection& visibleSelection, TextCheck
     // Since we couldn't check the entire block at once, set up starting and ending markers to fire incrementally.
     VisiblePosition startPos = visibleSelection.visibleStart();
     VisiblePosition startOfCurrentLine = startOfLine(startPos);
-    VisiblePosition endOfCurrentLine = endOfLine(startPos);
+    VisiblePosition endOfCurrentLine = endOfLine(startOfCurrentLine);
 
-    while (startOfCurrentLine != endOfCurrentLine) {
+    while (!isEndOfBlock(startOfCurrentLine)) {
         // Create a selection with the start and end points of the line, and convert to Range to create a SpellCheckRequest.
         rangeForSpellChecking = VisibleSelection(startOfCurrentLine, endOfCurrentLine).toNormalizedRange();
 
-        if (rangeForSpellChecking->text().length() >= MaxSpellCheckingStringLength) {
+        if (rangeForSpellChecking->text().length() < MaxSpellCheckingStringLength) {
+            startOfCurrentLine = nextLinePosition(startOfCurrentLine, startOfCurrentLine.lineDirectionPointForBlockDirectionNavigation());
+            endOfCurrentLine = endOfLine(startOfCurrentLine);
+        } else {
             // Iterate through words from the start of the line to the end.
             rangeForSpellChecking = getRangeForSpellCheckWithFineGranularity(startOfCurrentLine, endOfCurrentLine);
             if (!rangeForSpellChecking) {
@@ -890,12 +899,6 @@ void InputHandler::spellCheckBlock(VisibleSelection& visibleSelection, TextCheck
                 return;
             }
             startOfCurrentLine = VisiblePosition(rangeForSpellChecking->endPosition());
-        } else {
-            startOfCurrentLine = nextLinePosition(startOfCurrentLine, startOfCurrentLine.lineDirectionPointForBlockDirectionNavigation());
-            endOfCurrentLine = endOfLine(startOfCurrentLine);
-            // If we are at the last line, nextLinePosition will return the position at the end of the line. If we're not at the end, wrap with a call to startOfLine to be safe.
-            if (startOfCurrentLine != endOfCurrentLine)
-                startOfCurrentLine = startOfLine(startOfCurrentLine);
         }
 
         SpellingLog(LogLevelInfo, "InputHandler::spellCheckBlock Substring text is '%s', of size %d", rangeForSpellChecking->text().latin1().data(), rangeForSpellChecking->text().length());
index 869cfc2..56a66e2 100644 (file)
@@ -26,6 +26,7 @@
 #include <imf/events.h>
 #include <imf/input_data.h>
 #include <map>
+#include <pthread.h>
 #include <wtf/RefPtr.h>
 
 namespace WTF {
@@ -217,6 +218,7 @@ private:
     bool m_delayKeyboardVisibilityChange;
 
     std::map<int32_t, int32_t> m_sequenceMap;
+    pthread_mutex_t m_sequenceMapMutex;
 };
 
 }