Changeset 126883 in webkit

Timestamp:

Aug 28, 2012, 8:46:22 AM (14 years ago)

Author:

commit-queue@webkit.org

Message:

[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.

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

• WebKitSupport/InputHandler.cpp:

(BlackBerry::WebKit::InputHandler::spellCheckBlock):

Location:

trunk/Source/WebKit/blackberry

Files:

• ChangeLog (modified) (1 diff)
• WebKitSupport/InputHandler.cpp (modified) (9 diffs)
• WebKitSupport/InputHandler.h (modified) (2 diffs)

trunk/Source/WebKit/blackberry/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp

@@ -136 +136 @@
     , m_delayKeyboardVisibilityChange(false)
 {
+    pthread_mutex_init(&m_sequenceMapMutex, 0);
 }
 
@@ -582 +583 @@
     }
 
+    BlackBerry::Platform::MutexLocker lock(&m_sequenceMapMutex);
     int32_t transactionId = m_webPage->m_client->checkSpellingOfStringAsync(checkingString, paragraphLength);
     free(checkingString);
@@ -599 +601 @@
 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);
 
@@ -661 +664 @@
 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 */);
@@ -673 +677 @@
     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;
@@ -857 +861 @@
         return;
 
+    RefPtr<Range> rangeForSpellChecking = visibleSelection.toNormalizedRange();
+    if (!rangeForSpellChecking || !rangeForSpellChecking->text() || !rangeForSpellChecking->text().length())
+        return;
+
     SpellChecker* spellChecker = getSpellChecker();
     if (!spellChecker) {
@@ -862 +870 @@
         return;
     }
-
-    RefPtr<Range> rangeForSpellChecking = visibleSelection.toNormalizedRange();
 
     // If we have a batch request, try to send off the entire block.
@@ -877 +883 @@
     VisiblePosition startPos = visibleSelection.visibleStart();
     VisiblePosition startOfCurrentLine = startOfLine(startPos);
-    VisiblePosition endOfCurrentLine = endOfLine(startPos);
-
-    while (startOfCurrentLine != endOfCurrentLine) {
+    VisiblePosition endOfCurrentLine = endOfLine(startOfCurrentLine);
+
+    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);
@@ -891 +900 @@
             }
             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);
         }
 

trunk/Source/WebKit/blackberry/WebKitSupport/InputHandler.h

@@ -27 +27 @@
 #include <imf/input_data.h>
 #include <map>
+#include <pthread.h>
 #include <wtf/RefPtr.h>
 
@@ -218 +219 @@
 
     std::map<int32_t, int32_t> m_sequenceMap;
+    pthread_mutex_t m_sequenceMapMutex;
 };