Introduce TextFinder class for decoupling WebFrameImpl and text finder.

Introduce TextFinder class for decoupling WebFrameImpl and text finder.

It will be lazily initialized when user activates find on page functionality.

First part of this patch that adds TextIterator.{h,cpp} files is here:
https://codereview.chromium.org/181003010

BUG=348972
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170329

[Andrey Kraynov, 2014-02-27 16:11:07]

Initial patch was reverted in
https://codereview.chromium.org/132563003

because it broke Chromium test in interactive_ui_tests.
I've fixed my code that broke Chromium test, and I checked that all tests from
interactive_ui_tests are green (including FindInPageTest.FocusRestore).

Previous discussion for the patch is here 
https://codereview.chromium.org/69923006

Yoshi, would you check this code one more time, please?
Maybe should I check some additional tests to be sure that everything is
alright?

[yosin_UTC9, 2014-02-28 02:04:02]

I saw the pattern:
  ASSERT(m_textFinder);
  m_textFinder->function(...)

Can we change this to
  getOrCreateTextFinder().function(...)

Does this pattern make code fragile in future change? I'm not sure.

BTW, what did you change for fixing ui_interactive test failure?

-yosi

https://codereview.chromium.org/181013007/diff/1/Source/web/WebFrameImpl.cpp
File Source/web/WebFrameImpl.cpp (right):

https://codereview.chromium.org/181013007/diff/1/Source/web/WebFrameImpl.cpp#...
Source/web/WebFrameImpl.cpp:1532: ASSERT(m_textFinder);
We may want to keep |if (!isActiveMatchFrameValue()) ...|

https://codereview.chromium.org/181013007/diff/1/Source/web/WebFrameImpl.cpp#...
Source/web/WebFrameImpl.cpp:1779: if (this == mainFrameImpl->activeMatchFrame()
&& m_textFinder->activeMatch()) {
nit: How about below?

if (this != mainFrameImpl->activeMatchFrame())
  return;

if (Range* activeMatch = activeMatch()) {
 ...
}

https://codereview.chromium.org/181013007/diff/1/Source/web/WebFrameImpl.cpp#...
Source/web/WebFrameImpl.cpp:1927: ASSERT(m_textFinder);
nit: It seems this |ASSERT()| is redundant.

[Andrey Kraynov, 2014-03-05 16:14:28]

I've fixed remarks and added BUG id to link this issue with initial patch.

https://codereview.chromium.org/181013007/diff/1/Source/web/WebFrameImpl.cpp
File Source/web/WebFrameImpl.cpp (right):

https://codereview.chromium.org/181013007/diff/1/Source/web/WebFrameImpl.cpp#...
Source/web/WebFrameImpl.cpp:1532: ASSERT(m_textFinder);
On 2014/02/28 02:04:03, Yoshi wrote:
> We may want to keep |if (!isActiveMatchFrameValue()) ...|

Ok, I've removed ASSERT

https://codereview.chromium.org/181013007/diff/1/Source/web/WebFrameImpl.cpp#...
Source/web/WebFrameImpl.cpp:1779: if (this == mainFrameImpl->activeMatchFrame()
&& m_textFinder->activeMatch()) {
OK, fixed.

https://codereview.chromium.org/181013007/diff/1/Source/web/WebFrameImpl.cpp#...
Source/web/WebFrameImpl.cpp:1927: ASSERT(m_textFinder);
On 2014/02/28 02:04:03, Yoshi wrote:
> nit: It seems this |ASSERT()| is redundant.

Fixed.

[Andrey Kraynov, 2014-03-05 16:15:28]

On 2014/02/28 02:04:02, Yoshi wrote:
> I saw the pattern:
>   ASSERT(m_textFinder);
>   m_textFinder->function(...)
> 
> Can we change this to
>   getOrCreateTextFinder().function(...)

We could do it, but that ASSERT is our contract: functions like findMatchRects()
doesn’t make any sense if they are called before find() was called.

> 
> Does this pattern make code fragile in future change? I'm not sure.
Yes, this pattern definitely makes code more fragile =(
I think there are three ways to handle the situation:
- Make an ASSERT before calling m_textFinder methods, as I did.
- Do nothing if there is no valid m_textFinder object.
- Use getOrCreateTextFinder() and create m_textFinder if needed before any
operation.
  In this case we could just store it by value instead of OwnPtr<TextFinder> in
WebFrameImpl.
Each of these ways has its own advantages and disadvantages.
What do you think about it?

> 
> BTW, what did you change for fixing ui_interactive test failure?
> 
My fix was related exactly to the pattern that we discussed.
There was an ASSERT(m_textFinder) in WebFrameImpl::stopFinding, because I didn't
expect any call to this method before WebFrameImpl::find().
But that test calls stopFinding() first and then it fails.
So I modified code to the following:
if (m_textFinder) m_textFinder->stopFindingAndClearSelection()
instead of 
ASSERT(m_textFinder);
...
m_textFinder->stopFindingAndClearSelection();

[yosin_UTC9, 2014-03-10 01:48:19]

+tkent@ for OWNERS review
ACK

[tkent, 2014-03-10 03:34:42]

lgtm

[commit-bot: I haz the power, 2014-03-10 03:34:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iceman@yandex-team.ru/181013007/10001

[commit-bot: I haz the power, 2014-03-10 03:34:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-10 03:34:56]

Failed to apply patch for Source/web/TextFinder.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/web/TextFinder.cpp
  Hunk #1 FAILED at 28.
  Hunk #20 FAILED at 1929.
  Hunk #21 succeeded at 2323 (offset -3 lines).
  Hunk #22 succeeded at 2375 (offset -3 lines).
  Hunk #23 succeeded at 2389 (offset -3 lines).
  Hunk #24 succeeded at 2409 (offset -3 lines).
  2 out of 24 hunks FAILED -- saving rejects to file
Source/web/TextFinder.cpp.rej

Patch:       Source/web/TextFinder.cpp
Index: Source/web/TextFinder.cpp
diff --git a/Source/web/TextFinder.cpp b/Source/web/TextFinder.cpp
index
c871227ef61cade707ca6fa1fda08362d771563e..c1732cffec10c8f2b0447b22790216fcc6b27ffb
100644
--- a/Source/web/TextFinder.cpp
+++ b/Source/web/TextFinder.cpp
@@ -28,1497 +28,119 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-// How ownership works
-// -------------------
-//
-// Big oh represents a refcounted relationship: owner O--- ownee
-//
-// WebView (for the toplevel frame only)
-//    O
-//    |           WebFrame
-//    |              O
-//    |              |
-//   Page O------- LocalFrame (m_mainFrame) O-------O FrameView
-//                   ||
-//                   ||
-//               FrameLoader
-//
-// FrameLoader and LocalFrame are formerly one object that was split apart
because
-// it got too big. They basically have the same lifetime, hence the double
line.
-//
-// From the perspective of the embedder, WebFrame is simply an object that it
-// allocates by calling WebFrame::create() and must be freed by calling
close().
-// Internally, WebFrame is actually refcounted and it holds a reference to its
-// corresponding LocalFrame in WebCore.
-//
-// How frames are destroyed
-// ------------------------
-//
-// The main frame is never destroyed and is re-used. The FrameLoader is re-used
-// and a reference to the main frame is kept by the Page.
-//
-// When frame content is replaced, all subframes are destroyed. This happens
-// in FrameLoader::detachFromParent for each subframe in a pre-order
depth-first
-// traversal. Note that child node order may not match DOM node order!
-// detachFromParent() calls FrameLoaderClient::detachedFromParent(), which
calls
-// WebFrame::frameDetached(). This triggers WebFrame to clear its reference to
-// LocalFrame, and also notifies the embedder via WebFrameClient that the frame
is
-// detached. Most embedders will invoke close() on the WebFrame at this point,
-// triggering its deletion unless something else is still retaining a
reference.
-//
-// Thie client is expected to be set whenever the WebFrameImpl is attached to
-// the DOM.
 
 #include "config.h"
 #include "TextFinder.h"
 
-#include <algorithm>
-#include "AssociatedURLLoader.h"
-#include "DOMUtilitiesPrivate.h"
-#include "EventListenerWrapper.h"
 #include "FindInPageCoordinates.h"
-#include "HTMLNames.h"
-#include "PageOverlay.h"
-#include "SharedWorkerRepositoryClientImpl.h"
-#include "WebConsoleMessage.h"
-#include "WebDOMEvent.h"
-#include "WebDOMEventListener.h"
-#include "WebDataSourceImpl.h"
-#include "WebDevToolsAgentPrivate.h"
-#include "WebDocument.h"
 #include "WebFindOptions.h"
-#include "WebFormElement.h"
 #include "WebFrameClient.h"
-#include "WebHistoryItem.h"
-#include "WebIconURL.h"
-#include "WebInputElement.h"
-#include "WebNode.h"
-#include "WebPerformance.h"
-#include "WebPlugin.h"
-#include "WebPluginContainerImpl.h"
-#include "WebPrintParams.h"
-#include "WebRange.h"
-#include "WebScriptSource.h"
-#include "WebSecurityOrigin.h"
-#include "WebSerializedScriptValue.h"
+#include "WebFrameImpl.h"
+#include "WebViewClient.h"
 #include "WebViewImpl.h"
-#include "bindings/v8/DOMWrapperWorld.h"
-#include "bindings/v8/ExceptionState.h"
-#include "bindings/v8/ExceptionStatePlaceholder.h"
-#include "bindings/v8/ScriptController.h"
-#include "bindings/v8/ScriptSourceCode.h"
-#include "bindings/v8/ScriptValue.h"
-#include "bindings/v8/V8GCController.h"
-#include "bindings/v8/V8PerIsolateData.h"
-#include "core/dom/Document.h"
 #include "core/dom/DocumentMarker.h"
 #include "core/dom/DocumentMarkerController.h"
-#include "core/dom/IconURL.h"
-#include "core/dom/MessagePort.h"
-#include "core/dom/Node.h"
-#include "core/dom/NodeTraversal.h"
+#include "core/dom/Range.h"
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/editing/Editor.h"
-#include "core/editing/FrameSelection.h"
-#include "core/editing/InputMethodController.h"
-#include "core/editing/PlainTextRange.h"
-#include "core/editing/SpellChecker.h"
-#include "core/editing/TextAffinity.h"
 #include "core/editing/TextIterator.h"
-#include "core/editing/htmlediting.h"
-#include "core/editing/markup.h"
-#include "core/frame/Console.h"
-#include "core/frame/DOMWindow.h"
+#include "core/editing/VisibleSelection.h"
 #include "core/frame/FrameView.h"
-#include "core/html/HTMLCollection.h"
-#include "core/html/HTMLFormElement.h"
-#include "core/html/HTMLFrameOwnerElement.h"
-#include "core/html/HTMLHeadElement.h"
-#include "core/html/HTMLInputElement.h"
-#include "core/html/HTMLLinkElement.h"
-#include "core/html/PluginDocument.h"
-#include "core/inspector/InspectorController.h"
-#include "core/inspector/ScriptCallStack.h"
-#include "core/loader/DocumentLoader.h"
-#include "core/loader/FormState.h"
-#include "core/loader/FrameLoadRequest.h"
-#include "core/loader/FrameLoader.h"
-#include "core/loader/HistoryItem.h"
-#include "core/loader/SubstituteData.h"
-#include "core/page/Chrome.h"
-#include "core/page/EventHandler.h"
-#include "core/page/FocusController.h"
-#include "core/page/FrameTree.h"
-#include "core/page/Page.h"
-#include "core/page/PrintContext.h"
-#include "core/frame/Settings.h"
-#include "core/rendering/HitTestResult.h"
-#include "core/rendering/RenderBox.h"
-#include "core/rendering/RenderFrame.h"
-#include "core/rendering/RenderLayer.h"
-#include "core/rendering/RenderObject.h"
-#include "core/rendering/RenderTreeAsText.h"
-#include "core/rendering/RenderView.h"
-#include "core/rendering/style/StyleInheritedData.h"
-#include "core/timing/Performance.h"
-#include "core/xml/DocumentXPathEvaluator.h"
-#include "core/xml/XPathResult.h"
-#include "platform/TraceEvent.h"
-#include "platform/UserGestureIndicator.h"
-#include "platform/clipboard/ClipboardUtilities.h"
-#include "platform/fonts/FontCache.h"
-#include "platform/graphics/GraphicsContext.h"
-#include "platform/graphics/GraphicsLayerClient.h"
-#include "platform/graphics/skia/SkiaUtils.h"
-#include "platform/network/ResourceRequest.h"
-#include "platform/scroll/ScrollbarTheme.h"
-#include "platform/scroll/ScrollTypes.h"
-#include "platform/weborigin/KURL.h"
-#include "platform/weborigin/SchemeRegistry.h"
-#include "platform/weborigin/SecurityPolicy.h"
-#include "public/platform/Platform.h"
-#include "public/platform/WebFloatPoint.h"
-#include "public/platform/WebFloatRect.h"
-#include "public/platform/WebLayer.h"
-#include "public/platform/WebPoint.h"
-#include "public/platform/WebRect.h"
-#include "public/platform/WebSize.h"
-#include "public/platform/WebURLError.h"
+#include "platform/Timer.h"
 #include "public/platform/WebVector.h"
 #include "wtf/CurrentTime.h"
-#include "wtf/HashMap.h"
 
 using namespace WebCore;
 
 namespace blink {
 
-static int frameCount = 0;
-
-// Key for a StatsCounter tracking how many WebFrames are active.
-static const char webFrameActiveCount[] = "WebFrameActiveCount";
-
-static void frameContentAsPlainText(size_t maxChars, LocalFrame* frame,
StringBuilder& output)
-{
-    Document* document = frame->document();
-    if (!document)
-        return;
-
-    if (!frame->view())
-        return;
-
-    // TextIterator iterates over the visual representation of the DOM. As
such,
-    // it requires you to do a layout before using it (otherwise it'll crash).
-    document->updateLayout();
-
-    // Select the document body.
-    RefPtr<Range> range(document->createRange());
-    TrackExceptionState exceptionState;
-    range->selectNodeContents(document->body(), exceptionState);
-
-    if (!exceptionState.hadException()) {
-        // The text iterator will walk nodes giving us text. This is similar to
-        // the plainText() function in core/editing/TextIterator.h, but we
implement the maximum
-        // size and also copy the results directly into a wstring, avoiding the
-        // string conversion.
-        for (TextIterator it(range.get()); !it.atEnd(); it.advance()) {
-            it.appendTextToStringBuilder(output, 0, maxChars -
output.length());
-            if (output.length() >= maxChars)
-                return; // Filled up the buffer.
-        }
-    }
-
-    // The separator between frames when the frames are converted to plain
text.
-    const LChar frameSeparator[] = { '\n', '\n' };
-    const size_t frameSeparatorLength = WTF_ARRAY_LENGTH(frameSeparator);
-
-    // Recursively walk the children.
-    const FrameTree& frameTree = frame->tree();
-    for (LocalFrame* curChild = frameTree.firstChild(); curChild; curChild =
curChild->tree().nextSibling()) {
-        // Ignore the text of non-visible frames.
-        RenderView* contentRenderer = curChild->contentRenderer();
-        RenderPart* ownerRenderer = curChild->ownerRenderer();
-        if (!contentRenderer || !contentRenderer->width() ||
!contentRenderer->height()
-            || (contentRenderer->x() + contentRenderer->width() <= 0) ||
(contentRenderer->y() + contentRenderer->height() <= 0)
-            || (ownerRenderer && ownerRenderer->style() &&
ownerRenderer->style()->visibility() != VISIBLE)) {
-            continue;
-        }
-
-        // Make sure the frame separator won't fill up the buffer, and give up
if
-        // it will. The danger is if the separator will make the buffer longer
than
-        // maxChars. This will cause the computation above:
-        //   maxChars - output->size()
-        // to be a negative number which will crash when the subframe is added.
-     …
(message too large)

[Andrey Kraynov, 2014-03-10 18:21:32]

On 2014/03/10 03:34:56, I haz the power (commit-bot) wrote:
> Failed to apply patch for Source/web/TextFinder.cpp:
> While running patch -p1 --forward --force --no-backup-if-mismatch;
> (message too large)

I've rebased this CL.

[tkent, 2014-03-10 22:59:27]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2014-03-10 22:59:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iceman@yandex-team.ru/181013007/30001

[commit-bot: I haz the power, 2014-03-10 22:59:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-10 22:59:56]

Failed to apply patch for Source/web/WebFrameImpl.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/web/WebFrameImpl.cpp
  Hunk #2 FAILED at 451.
  1 out of 13 hunks FAILED -- saving rejects to file
Source/web/WebFrameImpl.cpp.rej

Patch:       Source/web/WebFrameImpl.cpp
Index: Source/web/WebFrameImpl.cpp
diff --git a/Source/web/WebFrameImpl.cpp b/Source/web/WebFrameImpl.cpp
index
4a1f80bbcee1f567ab855ad9d1193e18b814c748..5c40d86b70b0616f6ab8a8f19ae0436783827db4
100644
--- a/Source/web/WebFrameImpl.cpp
+++ b/Source/web/WebFrameImpl.cpp
@@ -80,6 +80,7 @@
 #include "HTMLNames.h"
 #include "PageOverlay.h"
 #include "SharedWorkerRepositoryClientImpl.h"
+#include "TextFinder.h"
 #include "WebConsoleMessage.h"
 #include "WebDOMEvent.h"
 #include "WebDOMEventListener.h"
@@ -450,39 +451,6 @@ static WebDataSource*
DataSourceForDocLoader(DocumentLoader* loader)
     return loader ? WebDataSourceImpl::fromDocumentLoader(loader) : 0;
 }
 
-WebFrameImpl::FindMatch::FindMatch(PassRefPtr<Range> range, int ordinal)
-    : m_range(range)
-    , m_ordinal(ordinal)
-{
-}
-
-class WebFrameImpl::DeferredScopeStringMatches {
-public:
-    DeferredScopeStringMatches(WebFrameImpl* webFrame, int identifier, const
WebString& searchText, const WebFindOptions& options, bool reset)
-        : m_timer(this, &DeferredScopeStringMatches::doTimeout)
-        , m_webFrame(webFrame)
-        , m_identifier(identifier)
-        , m_searchText(searchText)
-        , m_options(options)
-        , m_reset(reset)
-    {
-        m_timer.startOneShot(0.0);
-    }
-
-private:
-    void doTimeout(Timer<DeferredScopeStringMatches>*)
-    {
-        m_webFrame->callScopeStringMatches(this, m_identifier, m_searchText,
m_options, m_reset);
-    }
-
-    Timer<DeferredScopeStringMatches> m_timer;
-    RefPtr<WebFrameImpl> m_webFrame;
-    int m_identifier;
-    WebString m_searchText;
-    WebFindOptions m_options;
-    bool m_reset;
-};
-
 // WebFrame -------------------------------------------------------------------
 
 int WebFrame::instanceCount()
@@ -1468,344 +1436,41 @@ WebString WebFrameImpl::pageProperty(const WebString&
propertyName, int pageInde
 
 bool WebFrameImpl::find(int identifier, const WebString& searchText, const
WebFindOptions& options, bool wrapWithinFrame, WebRect* selectionRect)
 {
-    if (!frame() || !frame()->page())
-        return false;
-
-    WebFrameImpl* mainFrameImpl = viewImpl()->mainFrameImpl();
-
-    if (!options.findNext)
-        frame()->page()->unmarkAllTextMatches();
-    else
-        setMarkerActive(m_activeMatch.get(), false);
-
-    if (m_activeMatch && &m_activeMatch->ownerDocument() !=
frame()->document())
-        m_activeMatch = nullptr;
-
-    // If the user has selected something since the last Find operation we want
-    // to start from there. Otherwise, we start searching from where the last
Find
-    // operation left off (either a Find or a FindNext operation).
-    VisibleSelection selection(frame()->selection().selection());
-    bool activeSelection = !selection.isNone();
-    if (activeSelection) {
-        m_activeMatch = selection.firstRange().get();
-        frame()->selection().clear();
-    }
-
-    ASSERT(frame() && frame()->view());
-    const FindOptions findOptions = (options.forward ? 0 : Backwards)
-        | (options.matchCase ? 0 : CaseInsensitive)
-        | (wrapWithinFrame ? WrapAround : 0)
-        | (options.wordStart ? AtWordStarts : 0)
-        | (options.medialCapitalAsWordStart ? TreatMedialCapitalAsWordStart :
0)
-        | (options.findNext ? 0 : StartInSelection);
-    m_activeMatch = frame()->editor().findStringAndScrollToVisible(searchText,
m_activeMatch.get(), findOptions);
-
-    if (!m_activeMatch) {
-        // If we're finding next the next active match might not be in the
current frame.
-        // In this case we don't want to clear the matches cache.
-        if (!options.findNext)
-            clearFindMatchesCache();
-        invalidateArea(InvalidateAll);
-        return false;
-    }
-
-#if OS(ANDROID)
-   
viewImpl()->zoomToFindInPageRect(frameView()->contentsToWindow(enclosingIntRect(RenderObject::absoluteBoundingBoxRectForRange(m_activeMatch.get()))));
-#endif
-
-    setMarkerActive(m_activeMatch.get(), true);
-    WebFrameImpl* oldActiveFrame = mainFrameImpl->m_currentActiveMatchFrame;
-    mainFrameImpl->m_currentActiveMatchFrame = this;
-
-    // Make sure no node is focused. See http://crbug.com/38700.
-    frame()->document()->setFocusedElement(nullptr);
-
-    if (!options.findNext || activeSelection) {
-        // This is either a Find operation or a Find-next from a new start
point
-        // due to a selection, so we set the flag to ask the scoping effort
-        // to find the active rect for us and report it back to the UI.
-        m_locatingActiveRect = true;
-    } else {
-        if (oldActiveFrame != this) {
-            if (options.forward)
-                m_activeMatchIndexInCurrentFrame = 0;
-            else
-                m_activeMatchIndexInCurrentFrame = m_lastMatchCount - 1;
-        } else {
-            if (options.forward)
-                ++m_activeMatchIndexInCurrentFrame;
-            else
-                --m_activeMatchIndexInCurrentFrame;
-
-            if (m_activeMatchIndexInCurrentFrame + 1 > m_lastMatchCount)
-                m_activeMatchIndexInCurrentFrame = 0;
-            if (m_activeMatchIndexInCurrentFrame == -1)
-                m_activeMatchIndexInCurrentFrame = m_lastMatchCount - 1;
-        }
-        if (selectionRect) {
-            *selectionRect =
frameView()->contentsToWindow(m_activeMatch->boundingBox());
-            reportFindInPageSelection(*selectionRect,
m_activeMatchIndexInCurrentFrame + 1, identifier);
-        }
-    }
-
-    return true;
+    return getOrCreateTextFinder().find(identifier, searchText, options,
wrapWithinFrame, selectionRect);
 }
 
 void WebFrameImpl::stopFinding(bool clearSelection)
 {
-    if (!clearSelection)
-        setFindEndstateFocusAndSelection();
-    cancelPendingScopingEffort();
-
-    // Remove all markers for matches found and turn off the highlighting.
-    frame()->document()->markers().removeMarkers(DocumentMarker::TextMatch);
-    frame()->editor().setMarkedTextMatchesAreHighlighted(false);
-    clearFindMatchesCache();
-
-    // Let the frame know that we don't want tickmarks or highlighting anymore.
-    invalidateArea(InvalidateAll);
-}
-
-void WebFrameImpl::scopeStringMatches(int identifier, const WebString&
searchText, const WebFindOptions& options, bool reset)
-{
-    if (reset) {
-        // This is a brand new search, so we need to reset everything.
-        // Scoping is just about to begin.
-        m_scopingInProgress = true;
-
-        // Need to keep the current identifier locally in order to finish the
-        // request in case the frame is detached during the process.
-        m_findRequestIdentifier = identifier;
-
-        // Clear highlighting for this frame.
-        if (frame() && frame()->page() &&
frame()->editor().markedTextMatchesAreHighlighted())
-            frame()->page()->unmarkAllTextMatches();
-
-        // Clear the tickmarks and results cache.
-        clearFindMatchesCache();
-
-        // Clear the counters from last operation.
-        m_lastMatchCount = 0;
-        m_nextInvalidateAfter = 0;
-
-        m_resumeScopingFromRange = nullptr;
-
-        // The view might be null on detached frames.
-        if (frame() && frame()->page())
-            viewImpl()->mainFrameImpl()->m_framesScopingCount++;
-
-        // Now, defer scoping until later to allow find operation to finish
quickly.
-        scopeStringMatchesSoon(identifier, searchText, options, false); //
false means just reset, so don't do it again.
-        return;
+    if (m_textFinder) {
+        if (!clearSelection)
+            setFindEndstateFocusAndSelection();
+        m_textFinder->stopFindingAndClearSelection();
     }
-
-    if (!shouldScopeMatches(searchText)) {
-        // Note that we want to defer the final update when resetting even if
shouldScopeMatches returns false.
-        // This is done in order to prevent sending a final message based only
on the results of the first frame
-        // since m_framesScopingCount would be 0 as other frames have yet to
reset.
-        finishCurrentScopingEffort(identifier);
-        return;
-    }
-
-    WebFrameImpl* mainFrameImpl = viewImpl()->mainFrameImpl();
-    RefPtr<Range> searchRange(rangeOfContents(frame()->document()));
-
-    Node* originalEndContainer = searchRange->endContainer();
-    int originalEndOffset = searchRange->endOffset();
-
-    TrackExceptionState exceptionState, exceptionState2;
-    if (m_resumeScopingFromRange) {
-        // This is a continuation of a scoping operation that timed out and
didn't
-        // complete last time around, so we should start from where we left
off.
-        searchRange->setStart(m_resumeScopingFromRange->startContainer(),
m_resumeScopingFromRange->startOffset(exceptionState2) + 1, exceptionState);
-        if (exceptionState.hadException() || exceptionState2.hadException()) {
-            if (exceptionState2.hadException()) // A non-zero |exceptionState|
happens when navigating during search.
-                ASSERT_NOT_REACHED();
-            return;
-        }
-    }
-
-    // This timeout controls how long we scope before releasing control.  This
-    // value does not prevent us from running for longer than this, but it is
-    // periodically checked to see if we have exceeded our allocated time.
-    const double maxScopingDuration = 0.1; // seconds
-
-    int matchCount = 0;
-    bool timedOut = false;
-    double startTime = currentTime();
-    do {
-        // Find next occurrence of the search string.
-        // FIXME: (http://b/1088245) This WebKit operation may run for longer
-        // than the timeout value, and is not interruptible as it is currently
-        // writ…
(message too large)

[Andrey Kraynov, 2014-03-13 16:01:19]

The CQ bit was checked by iceman@yandex-team.ru

[commit-bot: I haz the power, 2014-03-13 16:01:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iceman@yandex-team.ru/181013007/50001

[Andrey Kraynov, 2014-03-13 16:14:14]

On 2014/03/10 22:59:56, I haz the power (commit-bot) wrote:
> Failed to apply patch for Source/web/WebFrameImpl.cpp:

I've rebased again.
Hope that it can be landed now.

[commit-bot: I haz the power, 2014-03-13 16:14:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-13 16:14:50]

Try jobs failed on following builders:
  tryserver.chromium on win_blink_compile

[Andrey Kraynov, 2014-03-13 17:29:27]

On 2014/03/13 16:14:50, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.chromium on win_blink_compile

I see that Linux build is broken at OwnPtr<T> and 'operator?'.
But on Windows I didn't get any errors at these lines.
Is this expected behavior?
Should I use OwnPtr<>::get() in 'operator?' ?

[tkent, 2014-03-14 00:20:35]

https://codereview.chromium.org/181013007/diff/50001/Source/web/WebFrameImpl.cpp
File Source/web/WebFrameImpl.cpp (right):

https://codereview.chromium.org/181013007/diff/50001/Source/web/WebFrameImpl....
Source/web/WebFrameImpl.cpp:1886: return m_textFinder ?
m_textFinder->activeMatchFrame() : nullptr;
I'm not sure exact reason of the compiler failure. Anyway, let's use if
(m_textFinder)... instead of ?:.

https://codereview.chromium.org/181013007/diff/50001/Source/web/WebFrameImpl....
Source/web/WebFrameImpl.cpp:1891: return m_textFinder ?
m_textFinder->activeMatch() : nullptr;
Ditto.

https://codereview.chromium.org/181013007/diff/50001/Source/web/WebFrameImpl.h
File Source/web/WebFrameImpl.h (right):

https://codereview.chromium.org/181013007/diff/50001/Source/web/WebFrameImpl....
Source/web/WebFrameImpl.h:304: TextFinder& getOrCreateTextFinder();
We usually name such function "ensureTextFinder()."

[Andrey Kraynov, 2014-03-17 17:56:26]

https://codereview.chromium.org/181013007/diff/50001/Source/web/WebFrameImpl.cpp
File Source/web/WebFrameImpl.cpp (right):

https://codereview.chromium.org/181013007/diff/50001/Source/web/WebFrameImpl....
Source/web/WebFrameImpl.cpp:1886: return m_textFinder ?
m_textFinder->activeMatchFrame() : nullptr;
On 2014/03/14 00:20:36, tkent wrote:
> I'm not sure exact reason of the compiler failure. Anyway, let's use if
> (m_textFinder)... instead of ?:.

OK, I've replaced ternary operator by if-statement.

https://codereview.chromium.org/181013007/diff/50001/Source/web/WebFrameImpl....
Source/web/WebFrameImpl.cpp:1891: return m_textFinder ?
m_textFinder->activeMatch() : nullptr;
On 2014/03/14 00:20:36, tkent wrote:
> Ditto.

Done.

https://codereview.chromium.org/181013007/diff/50001/Source/web/WebFrameImpl.h
File Source/web/WebFrameImpl.h (right):

https://codereview.chromium.org/181013007/diff/50001/Source/web/WebFrameImpl....
Source/web/WebFrameImpl.h:304: TextFinder& getOrCreateTextFinder();
On 2014/03/14 00:20:36, tkent wrote:
> We usually name such function "ensureTextFinder()."

Thanks for the tip.
I've renamed this function.

[tkent, 2014-03-17 19:37:01]

lgtm

[commit-bot: I haz the power, 2014-03-17 19:37:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iceman@yandex-team.ru/181013007/110001

[commit-bot: I haz the power, 2014-03-17 19:37:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-17 19:37:46]

Failed to apply patch for Source/web/TextFinder.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/web/TextFinder.cpp
  Hunk #20 FAILED at 551.
  Hunk #21 succeeded at 946 (offset 1 line).
  Hunk #22 succeeded at 998 (offset 1 line).
  Hunk #23 succeeded at 1012 (offset 1 line).
  Hunk #24 succeeded at 1032 (offset 1 line).
  1 out of 24 hunks FAILED -- saving rejects to file
Source/web/TextFinder.cpp.rej

Patch:       Source/web/TextFinder.cpp
Index: Source/web/TextFinder.cpp
diff --git a/Source/web/TextFinder.cpp b/Source/web/TextFinder.cpp
index
2cfc3dd6c4eea3d03d51c515f1ae9b2d35b6ae89..51ac3db94c06754d0639688e092e91553913d2ef
100644
--- a/Source/web/TextFinder.cpp
+++ b/Source/web/TextFinder.cpp
@@ -28,1496 +28,119 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-// How ownership works
-// -------------------
-//
-// Big oh represents a refcounted relationship: owner O--- ownee
-//
-// WebView (for the toplevel frame only)
-//    O
-//    |           WebFrame
-//    |              O
-//    |              |
-//   Page O------- LocalFrame (m_mainFrame) O-------O FrameView
-//                   ||
-//                   ||
-//               FrameLoader
-//
-// FrameLoader and LocalFrame are formerly one object that was split apart
because
-// it got too big. They basically have the same lifetime, hence the double
line.
-//
-// From the perspective of the embedder, WebFrame is simply an object that it
-// allocates by calling WebFrame::create() and must be freed by calling
close().
-// Internally, WebFrame is actually refcounted and it holds a reference to its
-// corresponding LocalFrame in WebCore.
-//
-// How frames are destroyed
-// ------------------------
-//
-// The main frame is never destroyed and is re-used. The FrameLoader is re-used
-// and a reference to the main frame is kept by the Page.
-//
-// When frame content is replaced, all subframes are destroyed. This happens
-// in FrameLoader::detachFromParent for each subframe in a pre-order
depth-first
-// traversal. Note that child node order may not match DOM node order!
-// detachFromParent() calls FrameLoaderClient::detachedFromParent(), which
calls
-// WebFrame::frameDetached(). This triggers WebFrame to clear its reference to
-// LocalFrame, and also notifies the embedder via WebFrameClient that the frame
is
-// detached. Most embedders will invoke close() on the WebFrame at this point,
-// triggering its deletion unless something else is still retaining a
reference.
-//
-// Thie client is expected to be set whenever the WebFrameImpl is attached to
-// the DOM.
 
 #include "config.h"
 #include "TextFinder.h"
 
-#include <algorithm>
-#include "AssociatedURLLoader.h"
-#include "DOMUtilitiesPrivate.h"
-#include "EventListenerWrapper.h"
 #include "FindInPageCoordinates.h"
-#include "HTMLNames.h"
-#include "PageOverlay.h"
-#include "SharedWorkerRepositoryClientImpl.h"
-#include "WebConsoleMessage.h"
-#include "WebDOMEvent.h"
-#include "WebDOMEventListener.h"
-#include "WebDataSourceImpl.h"
-#include "WebDevToolsAgentPrivate.h"
-#include "WebDocument.h"
 #include "WebFindOptions.h"
-#include "WebFormElement.h"
 #include "WebFrameClient.h"
-#include "WebHistoryItem.h"
-#include "WebIconURL.h"
-#include "WebInputElement.h"
-#include "WebNode.h"
-#include "WebPerformance.h"
-#include "WebPlugin.h"
-#include "WebPluginContainerImpl.h"
-#include "WebPrintParams.h"
-#include "WebRange.h"
-#include "WebScriptSource.h"
-#include "WebSecurityOrigin.h"
-#include "WebSerializedScriptValue.h"
+#include "WebFrameImpl.h"
+#include "WebViewClient.h"
 #include "WebViewImpl.h"
-#include "bindings/v8/DOMWrapperWorld.h"
-#include "bindings/v8/ExceptionState.h"
-#include "bindings/v8/ExceptionStatePlaceholder.h"
-#include "bindings/v8/ScriptController.h"
-#include "bindings/v8/ScriptSourceCode.h"
-#include "bindings/v8/ScriptValue.h"
-#include "bindings/v8/V8GCController.h"
-#include "bindings/v8/V8PerIsolateData.h"
-#include "core/dom/Document.h"
 #include "core/dom/DocumentMarker.h"
 #include "core/dom/DocumentMarkerController.h"
-#include "core/dom/IconURL.h"
-#include "core/dom/MessagePort.h"
-#include "core/dom/Node.h"
-#include "core/dom/NodeTraversal.h"
+#include "core/dom/Range.h"
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/editing/Editor.h"
-#include "core/editing/FrameSelection.h"
-#include "core/editing/InputMethodController.h"
-#include "core/editing/PlainTextRange.h"
-#include "core/editing/SpellChecker.h"
-#include "core/editing/TextAffinity.h"
 #include "core/editing/TextIterator.h"
-#include "core/editing/htmlediting.h"
-#include "core/editing/markup.h"
-#include "core/frame/Console.h"
-#include "core/frame/DOMWindow.h"
+#include "core/editing/VisibleSelection.h"
 #include "core/frame/FrameView.h"
-#include "core/html/HTMLCollection.h"
-#include "core/html/HTMLFormElement.h"
-#include "core/html/HTMLFrameOwnerElement.h"
-#include "core/html/HTMLHeadElement.h"
-#include "core/html/HTMLInputElement.h"
-#include "core/html/HTMLLinkElement.h"
-#include "core/html/PluginDocument.h"
-#include "core/inspector/InspectorController.h"
-#include "core/inspector/ScriptCallStack.h"
-#include "core/loader/DocumentLoader.h"
-#include "core/loader/FrameLoadRequest.h"
-#include "core/loader/FrameLoader.h"
-#include "core/loader/HistoryItem.h"
-#include "core/loader/SubstituteData.h"
-#include "core/page/Chrome.h"
-#include "core/page/EventHandler.h"
-#include "core/page/FocusController.h"
-#include "core/page/FrameTree.h"
-#include "core/page/Page.h"
-#include "core/page/PrintContext.h"
-#include "core/frame/Settings.h"
-#include "core/rendering/HitTestResult.h"
-#include "core/rendering/RenderBox.h"
-#include "core/rendering/RenderFrame.h"
-#include "core/rendering/RenderLayer.h"
-#include "core/rendering/RenderObject.h"
-#include "core/rendering/RenderTreeAsText.h"
-#include "core/rendering/RenderView.h"
-#include "core/rendering/style/StyleInheritedData.h"
-#include "core/timing/Performance.h"
-#include "core/xml/DocumentXPathEvaluator.h"
-#include "core/xml/XPathResult.h"
-#include "platform/TraceEvent.h"
-#include "platform/UserGestureIndicator.h"
-#include "platform/clipboard/ClipboardUtilities.h"
-#include "platform/fonts/FontCache.h"
-#include "platform/graphics/GraphicsContext.h"
-#include "platform/graphics/GraphicsLayerClient.h"
-#include "platform/graphics/skia/SkiaUtils.h"
-#include "platform/network/ResourceRequest.h"
-#include "platform/scroll/ScrollbarTheme.h"
-#include "platform/scroll/ScrollTypes.h"
-#include "platform/weborigin/KURL.h"
-#include "platform/weborigin/SchemeRegistry.h"
-#include "platform/weborigin/SecurityPolicy.h"
-#include "public/platform/Platform.h"
-#include "public/platform/WebFloatPoint.h"
-#include "public/platform/WebFloatRect.h"
-#include "public/platform/WebLayer.h"
-#include "public/platform/WebPoint.h"
-#include "public/platform/WebRect.h"
-#include "public/platform/WebSize.h"
-#include "public/platform/WebURLError.h"
+#include "platform/Timer.h"
 #include "public/platform/WebVector.h"
 #include "wtf/CurrentTime.h"
-#include "wtf/HashMap.h"
 
 using namespace WebCore;
 
 namespace blink {
 
-static int frameCount = 0;
-
-// Key for a StatsCounter tracking how many WebFrames are active.
-static const char webFrameActiveCount[] = "WebFrameActiveCount";
-
-static void frameContentAsPlainText(size_t maxChars, LocalFrame* frame,
StringBuilder& output)
-{
-    Document* document = frame->document();
-    if (!document)
-        return;
-
-    if (!frame->view())
-        return;
-
-    // TextIterator iterates over the visual representation of the DOM. As
such,
-    // it requires you to do a layout before using it (otherwise it'll crash).
-    document->updateLayout();
-
-    // Select the document body.
-    RefPtr<Range> range(document->createRange());
-    TrackExceptionState exceptionState;
-    range->selectNodeContents(document->body(), exceptionState);
-
-    if (!exceptionState.hadException()) {
-        // The text iterator will walk nodes giving us text. This is similar to
-        // the plainText() function in core/editing/TextIterator.h, but we
implement the maximum
-        // size and also copy the results directly into a wstring, avoiding the
-        // string conversion.
-        for (TextIterator it(range.get()); !it.atEnd(); it.advance()) {
-            it.appendTextToStringBuilder(output, 0, maxChars -
output.length());
-            if (output.length() >= maxChars)
-                return; // Filled up the buffer.
-        }
-    }
-
-    // The separator between frames when the frames are converted to plain
text.
-    const LChar frameSeparator[] = { '\n', '\n' };
-    const size_t frameSeparatorLength = WTF_ARRAY_LENGTH(frameSeparator);
-
-    // Recursively walk the children.
-    const FrameTree& frameTree = frame->tree();
-    for (LocalFrame* curChild = frameTree.firstChild(); curChild; curChild =
curChild->tree().nextSibling()) {
-        // Ignore the text of non-visible frames.
-        RenderView* contentRenderer = curChild->contentRenderer();
-        RenderPart* ownerRenderer = curChild->ownerRenderer();
-        if (!contentRenderer || !contentRenderer->width() ||
!contentRenderer->height()
-            || (contentRenderer->x() + contentRenderer->width() <= 0) ||
(contentRenderer->y() + contentRenderer->height() <= 0)
-            || (ownerRenderer && ownerRenderer->style() &&
ownerRenderer->style()->visibility() != VISIBLE)) {
-            continue;
-        }
-
-        // Make sure the frame separator won't fill up the buffer, and give up
if
-        // it will. The danger is if the separator will make the buffer longer
than
-        // maxChars. This will cause the computation above:
-        //   maxChars - output->size()
-        // to be a negative number which will crash when the subframe is added.
-        if (output.length() >= maxChars - frameSeparatorLength)
-           …
(message too large)

[Andrey Kraynov, 2014-03-18 17:13:51]

The CQ bit was checked by iceman@yandex-team.ru

[commit-bot: I haz the power, 2014-03-18 17:14:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iceman@yandex-team.ru/181013007/130001

[commit-bot: I haz the power, 2014-03-18 17:14:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-18 17:14:37]

Failed to apply patch for Source/web/TextFinder.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/web/TextFinder.cpp
  Hunk #20 FAILED at 551.
  1 out of 24 hunks FAILED -- saving rejects to file
Source/web/TextFinder.cpp.rej

Patch:       Source/web/TextFinder.cpp
Index: Source/web/TextFinder.cpp
diff --git a/Source/web/TextFinder.cpp b/Source/web/TextFinder.cpp
index
a82113dc6a13561085255fe25f63bd083b32fe3b..51ac3db94c06754d0639688e092e91553913d2ef
100644
--- a/Source/web/TextFinder.cpp
+++ b/Source/web/TextFinder.cpp
@@ -28,1496 +28,119 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-// How ownership works
-// -------------------
-//
-// Big oh represents a refcounted relationship: owner O--- ownee
-//
-// WebView (for the toplevel frame only)
-//    O
-//    |           WebFrame
-//    |              O
-//    |              |
-//   Page O------- LocalFrame (m_mainFrame) O-------O FrameView
-//                   ||
-//                   ||
-//               FrameLoader
-//
-// FrameLoader and LocalFrame are formerly one object that was split apart
because
-// it got too big. They basically have the same lifetime, hence the double
line.
-//
-// From the perspective of the embedder, WebFrame is simply an object that it
-// allocates by calling WebFrame::create() and must be freed by calling
close().
-// Internally, WebFrame is actually refcounted and it holds a reference to its
-// corresponding LocalFrame in WebCore.
-//
-// How frames are destroyed
-// ------------------------
-//
-// The main frame is never destroyed and is re-used. The FrameLoader is re-used
-// and a reference to the main frame is kept by the Page.
-//
-// When frame content is replaced, all subframes are destroyed. This happens
-// in FrameLoader::detachFromParent for each subframe in a pre-order
depth-first
-// traversal. Note that child node order may not match DOM node order!
-// detachFromParent() calls FrameLoaderClient::detachedFromParent(), which
calls
-// WebFrame::frameDetached(). This triggers WebFrame to clear its reference to
-// LocalFrame, and also notifies the embedder via WebFrameClient that the frame
is
-// detached. Most embedders will invoke close() on the WebFrame at this point,
-// triggering its deletion unless something else is still retaining a
reference.
-//
-// Thie client is expected to be set whenever the WebFrameImpl is attached to
-// the DOM.
 
 #include "config.h"
 #include "TextFinder.h"
 
-#include <algorithm>
-#include "AssociatedURLLoader.h"
-#include "DOMUtilitiesPrivate.h"
-#include "EventListenerWrapper.h"
 #include "FindInPageCoordinates.h"
-#include "HTMLNames.h"
-#include "PageOverlay.h"
-#include "SharedWorkerRepositoryClientImpl.h"
-#include "WebConsoleMessage.h"
-#include "WebDOMEvent.h"
-#include "WebDOMEventListener.h"
-#include "WebDataSourceImpl.h"
-#include "WebDevToolsAgentPrivate.h"
-#include "WebDocument.h"
 #include "WebFindOptions.h"
-#include "WebFormElement.h"
 #include "WebFrameClient.h"
-#include "WebHistoryItem.h"
-#include "WebIconURL.h"
-#include "WebInputElement.h"
-#include "WebNode.h"
-#include "WebPerformance.h"
-#include "WebPlugin.h"
-#include "WebPluginContainerImpl.h"
-#include "WebPrintParams.h"
-#include "WebRange.h"
-#include "WebScriptSource.h"
-#include "WebSecurityOrigin.h"
-#include "WebSerializedScriptValue.h"
+#include "WebFrameImpl.h"
+#include "WebViewClient.h"
 #include "WebViewImpl.h"
-#include "bindings/v8/DOMWrapperWorld.h"
-#include "bindings/v8/ExceptionState.h"
-#include "bindings/v8/ExceptionStatePlaceholder.h"
-#include "bindings/v8/ScriptController.h"
-#include "bindings/v8/ScriptSourceCode.h"
-#include "bindings/v8/ScriptValue.h"
-#include "bindings/v8/V8GCController.h"
-#include "bindings/v8/V8PerIsolateData.h"
-#include "core/dom/Document.h"
 #include "core/dom/DocumentMarker.h"
 #include "core/dom/DocumentMarkerController.h"
-#include "core/dom/IconURL.h"
-#include "core/dom/MessagePort.h"
-#include "core/dom/Node.h"
-#include "core/dom/NodeTraversal.h"
+#include "core/dom/Range.h"
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/editing/Editor.h"
-#include "core/editing/FrameSelection.h"
-#include "core/editing/InputMethodController.h"
-#include "core/editing/PlainTextRange.h"
-#include "core/editing/SpellChecker.h"
-#include "core/editing/TextAffinity.h"
 #include "core/editing/TextIterator.h"
-#include "core/editing/htmlediting.h"
-#include "core/editing/markup.h"
-#include "core/frame/Console.h"
-#include "core/frame/DOMWindow.h"
+#include "core/editing/VisibleSelection.h"
 #include "core/frame/FrameView.h"
-#include "core/html/HTMLCollection.h"
-#include "core/html/HTMLFormElement.h"
-#include "core/html/HTMLFrameOwnerElement.h"
-#include "core/html/HTMLHeadElement.h"
-#include "core/html/HTMLInputElement.h"
-#include "core/html/HTMLLinkElement.h"
-#include "core/html/PluginDocument.h"
-#include "core/inspector/InspectorController.h"
-#include "core/inspector/ScriptCallStack.h"
-#include "core/loader/DocumentLoader.h"
-#include "core/loader/FrameLoadRequest.h"
-#include "core/loader/FrameLoader.h"
-#include "core/loader/HistoryItem.h"
-#include "core/loader/SubstituteData.h"
-#include "core/page/Chrome.h"
-#include "core/page/EventHandler.h"
-#include "core/page/FocusController.h"
-#include "core/page/FrameTree.h"
-#include "core/page/Page.h"
-#include "core/page/PrintContext.h"
-#include "core/frame/Settings.h"
-#include "core/rendering/HitTestResult.h"
-#include "core/rendering/RenderBox.h"
-#include "core/rendering/RenderFrame.h"
-#include "core/rendering/RenderLayer.h"
-#include "core/rendering/RenderObject.h"
-#include "core/rendering/RenderTreeAsText.h"
-#include "core/rendering/RenderView.h"
-#include "core/rendering/style/StyleInheritedData.h"
-#include "core/timing/Performance.h"
-#include "core/xml/DocumentXPathEvaluator.h"
-#include "core/xml/XPathResult.h"
-#include "platform/TraceEvent.h"
-#include "platform/UserGestureIndicator.h"
-#include "platform/clipboard/ClipboardUtilities.h"
-#include "platform/fonts/FontCache.h"
-#include "platform/graphics/GraphicsContext.h"
-#include "platform/graphics/GraphicsLayerClient.h"
-#include "platform/graphics/skia/SkiaUtils.h"
-#include "platform/network/ResourceRequest.h"
-#include "platform/scroll/ScrollbarTheme.h"
-#include "platform/scroll/ScrollTypes.h"
-#include "platform/weborigin/KURL.h"
-#include "platform/weborigin/SchemeRegistry.h"
-#include "platform/weborigin/SecurityPolicy.h"
-#include "public/platform/Platform.h"
-#include "public/platform/WebFloatPoint.h"
-#include "public/platform/WebFloatRect.h"
-#include "public/platform/WebLayer.h"
-#include "public/platform/WebPoint.h"
-#include "public/platform/WebRect.h"
-#include "public/platform/WebSize.h"
-#include "public/platform/WebURLError.h"
+#include "platform/Timer.h"
 #include "public/platform/WebVector.h"
 #include "wtf/CurrentTime.h"
-#include "wtf/HashMap.h"
 
 using namespace WebCore;
 
 namespace blink {
 
-static int frameCount = 0;
-
-// Key for a StatsCounter tracking how many WebFrames are active.
-static const char webFrameActiveCount[] = "WebFrameActiveCount";
-
-static void frameContentAsPlainText(size_t maxChars, LocalFrame* frame,
StringBuilder& output)
-{
-    Document* document = frame->document();
-    if (!document)
-        return;
-
-    if (!frame->view())
-        return;
-
-    // TextIterator iterates over the visual representation of the DOM. As
such,
-    // it requires you to do a layout before using it (otherwise it'll crash).
-    document->updateLayout();
-
-    // Select the document body.
-    RefPtr<Range> range(document->createRange());
-    TrackExceptionState exceptionState;
-    range->selectNodeContents(document->body(), exceptionState);
-
-    if (!exceptionState.hadException()) {
-        // The text iterator will walk nodes giving us text. This is similar to
-        // the plainText() function in core/editing/TextIterator.h, but we
implement the maximum
-        // size and also copy the results directly into a wstring, avoiding the
-        // string conversion.
-        for (TextIterator it(range.get()); !it.atEnd(); it.advance()) {
-            it.appendTextToStringBuilder(output, 0, maxChars -
output.length());
-            if (output.length() >= maxChars)
-                return; // Filled up the buffer.
-        }
-    }
-
-    // The separator between frames when the frames are converted to plain
text.
-    const LChar frameSeparator[] = { '\n', '\n' };
-    const size_t frameSeparatorLength = WTF_ARRAY_LENGTH(frameSeparator);
-
-    // Recursively walk the children.
-    const FrameTree& frameTree = frame->tree();
-    for (LocalFrame* curChild = frameTree.firstChild(); curChild; curChild =
curChild->tree().nextSibling()) {
-        // Ignore the text of non-visible frames.
-        RenderView* contentRenderer = curChild->contentRenderer();
-        RenderPart* ownerRenderer = curChild->ownerRenderer();
-        if (!contentRenderer || !contentRenderer->width() ||
!contentRenderer->height()
-            || (contentRenderer->x() + contentRenderer->width() <= 0) ||
(contentRenderer->y() + contentRenderer->height() <= 0)
-            || (ownerRenderer && ownerRenderer->style() &&
ownerRenderer->style()->visibility() != VISIBLE)) {
-            continue;
-        }
-
-        // Make sure the frame separator won't fill up the buffer, and give up
if
-        // it will. The danger is if the separator will make the buffer longer
than
-        // maxChars. This will cause the computation above:
-        //   maxChars - output->size()
-        // to be a negative number which will crash when the subframe is added.
-        if (output.length() >= maxChars - frameSeparatorLength)
-            return;
-
-        output.append(frameSeparator, frameSeparatorLength);
-        frameContentAsPlainText(maxChars, curChild, output);
-        if (output.length() >= maxChars)
-    …
(message too large)

[Andrey Kraynov, 2014-03-23 21:00:23]

On 2014/03/18 17:14:37, I haz the power (commit-bot) wrote:
> Failed to apply patch for Source/web/TextFinder.cpp:

Earlier I have moved function invalidateArea() and enum AreaToInvalidate from
class WebFrameImpl to class TextFinder, because TextFinder shouldn't be a friend
to WebFrameImpl but it needs to call invalidateArea.
Now I realized that it is not a good idea.
There is a CL https://codereview.chromium.org/198683002 that started to use
invalidateArea function from WebFrameImpl::setTickmarks function.
That's why my patch can't be applied.
That function invalidateArea needs to be called from WebFrameImpl class and from
TextFinder class.

I think that there are three ways to solve this problem:
 - Make TextFinder a friend of WebFrameImpl class.
 - Make invalidateArea a public function of WebFrameImpl class.
 - Create a delegate class like 'class WebFrameInvalidateDelegate { virtual void
invalidateArea() = 0; };'
  And send it to TextFinder object for WebFrameImpl invalidation.

I have chosen the second one.
Values of enum AreaToInvalidate are never used together, so I removed the enum
and created two distinct functions, invalidateAll() and invalidateScrollbar().

Is this solution acceptable? If not, please give me an advice.

[yosin_UTC9, 2014-03-24 02:05:24]

On 2014/03/23 21:00:23, iceman wrote:
> On 2014/03/18 17:14:37, I haz the power (commit-bot) wrote:
> > Failed to apply patch for Source/web/TextFinder.cpp:
> 
> Earlier I have moved function invalidateArea() and enum AreaToInvalidate from
> class WebFrameImpl to class TextFinder, because TextFinder shouldn't be a
friend
> to WebFrameImpl but it needs to call invalidateArea.
> Now I realized that it is not a good idea.
> There is a CL https://codereview.chromium.org/198683002 that started to use
> invalidateArea function from WebFrameImpl::setTickmarks function.
> That's why my patch can't be applied.
> That function invalidateArea needs to be called from WebFrameImpl class and
from
> TextFinder class.
> 
> I think that there are three ways to solve this problem:
>  - Make TextFinder a friend of WebFrameImpl class.
>  - Make invalidateArea a public function of WebFrameImpl class.
>  - Create a delegate class like 'class WebFrameInvalidateDelegate { virtual
void
> invalidateArea() = 0; };'
>   And send it to TextFinder object for WebFrameImpl invalidation.
> 
> I have chosen the second one.
> Values of enum AreaToInvalidate are never used together, so I removed the enum
> and created two distinct functions, invalidateAll() and invalidateScrollbar().
> 
> Is this solution acceptable? If not, please give me an advice.

I prefer:
 - having invalidateAll and invalidateScrollBar. It seems implementations of
them are independent.
 - make invalidateAll and invalidateScrollBar public rather than friend to avoid
implementation leak from WebFrameImpl.

Thanks.
- yosi

[Andrey Kraynov, 2014-03-24 16:34:07]

OK, I've divided one function invalidateArea into two distinct functions.

https://codereview.chromium.org/181013007/diff/150001/Source/web/WebFrameImpl...
File Source/web/WebFrameImpl.cpp (right):

https://codereview.chromium.org/181013007/diff/150001/Source/web/WebFrameImpl...
Source/web/WebFrameImpl.cpp:1937: invalidateScrollbar();
invalidateAll function calls invalidateScrollbar internally.

https://codereview.chromium.org/181013007/diff/150001/Source/web/WebFrameImpl.h
File Source/web/WebFrameImpl.h (right):

https://codereview.chromium.org/181013007/diff/150001/Source/web/WebFrameImpl...
Source/web/WebFrameImpl.h:309: void invalidateScrollbar() const;
Here are these new functions, invalidateScrollbar and invalidateAll.

[tkent, 2014-03-25 21:55:20]

lgtm

[commit-bot: I haz the power, 2014-03-25 21:55:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iceman@yandex-team.ru/181013007/150001

[commit-bot: I haz the power, 2014-03-25 21:55:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 21:55:35]

Failed to apply patch for Source/web/TextFinder.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/web/TextFinder.cpp
  Hunk #1 FAILED at 28.
  1 out of 23 hunks FAILED -- saving rejects to file
Source/web/TextFinder.cpp.rej

Patch:       Source/web/TextFinder.cpp
Index: Source/web/TextFinder.cpp
diff --git a/Source/web/TextFinder.cpp b/Source/web/TextFinder.cpp
index
a9873e0f608b5bd559f47631041bc89ac0c9a26f..27b9c6333023ba897af6815f4b2671ca9458173d
100644
--- a/Source/web/TextFinder.cpp
+++ b/Source/web/TextFinder.cpp
@@ -28,1496 +28,119 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-// How ownership works
-// -------------------
-//
-// Big oh represents a refcounted relationship: owner O--- ownee
-//
-// WebView (for the toplevel frame only)
-//    O
-//    |           WebFrame
-//    |              O
-//    |              |
-//   Page O------- LocalFrame (m_mainFrame) O-------O FrameView
-//                   ||
-//                   ||
-//               FrameLoader
-//
-// FrameLoader and LocalFrame are formerly one object that was split apart
because
-// it got too big. They basically have the same lifetime, hence the double
line.
-//
-// From the perspective of the embedder, WebFrame is simply an object that it
-// allocates by calling WebFrame::create() and must be freed by calling
close().
-// Internally, WebFrame is actually refcounted and it holds a reference to its
-// corresponding LocalFrame in WebCore.
-//
-// How frames are destroyed
-// ------------------------
-//
-// The main frame is never destroyed and is re-used. The FrameLoader is re-used
-// and a reference to the main frame is kept by the Page.
-//
-// When frame content is replaced, all subframes are destroyed. This happens
-// in FrameLoader::detachFromParent for each subframe in a pre-order
depth-first
-// traversal. Note that child node order may not match DOM node order!
-// detachFromParent() calls FrameLoaderClient::detachedFromParent(), which
calls
-// WebFrame::frameDetached(). This triggers WebFrame to clear its reference to
-// LocalFrame, and also notifies the embedder via WebFrameClient that the frame
is
-// detached. Most embedders will invoke close() on the WebFrame at this point,
-// triggering its deletion unless something else is still retaining a
reference.
-//
-// Thie client is expected to be set whenever the WebFrameImpl is attached to
-// the DOM.
 
 #include "config.h"
 #include "TextFinder.h"
 
-#include <algorithm>
-#include "AssociatedURLLoader.h"
-#include "DOMUtilitiesPrivate.h"
-#include "EventListenerWrapper.h"
 #include "FindInPageCoordinates.h"
-#include "HTMLNames.h"
-#include "PageOverlay.h"
-#include "SharedWorkerRepositoryClientImpl.h"
-#include "WebConsoleMessage.h"
-#include "WebDOMEvent.h"
-#include "WebDOMEventListener.h"
-#include "WebDataSourceImpl.h"
-#include "WebDevToolsAgentPrivate.h"
-#include "WebDocument.h"
 #include "WebFindOptions.h"
-#include "WebFormElement.h"
 #include "WebFrameClient.h"
-#include "WebHistoryItem.h"
-#include "WebIconURL.h"
-#include "WebInputElement.h"
-#include "WebNode.h"
-#include "WebPerformance.h"
-#include "WebPlugin.h"
-#include "WebPluginContainerImpl.h"
-#include "WebPrintParams.h"
-#include "WebRange.h"
-#include "WebScriptSource.h"
-#include "WebSecurityOrigin.h"
-#include "WebSerializedScriptValue.h"
+#include "WebFrameImpl.h"
+#include "WebViewClient.h"
 #include "WebViewImpl.h"
-#include "bindings/v8/DOMWrapperWorld.h"
-#include "bindings/v8/ExceptionState.h"
-#include "bindings/v8/ExceptionStatePlaceholder.h"
-#include "bindings/v8/ScriptController.h"
-#include "bindings/v8/ScriptSourceCode.h"
-#include "bindings/v8/ScriptValue.h"
-#include "bindings/v8/V8GCController.h"
-#include "bindings/v8/V8PerIsolateData.h"
-#include "core/dom/Document.h"
 #include "core/dom/DocumentMarker.h"
 #include "core/dom/DocumentMarkerController.h"
-#include "core/dom/IconURL.h"
-#include "core/dom/MessagePort.h"
-#include "core/dom/Node.h"
-#include "core/dom/NodeTraversal.h"
+#include "core/dom/Range.h"
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/editing/Editor.h"
-#include "core/editing/FrameSelection.h"
-#include "core/editing/InputMethodController.h"
-#include "core/editing/PlainTextRange.h"
-#include "core/editing/SpellChecker.h"
-#include "core/editing/TextAffinity.h"
 #include "core/editing/TextIterator.h"
-#include "core/editing/htmlediting.h"
-#include "core/editing/markup.h"
-#include "core/frame/Console.h"
-#include "core/frame/DOMWindow.h"
+#include "core/editing/VisibleSelection.h"
 #include "core/frame/FrameView.h"
-#include "core/html/HTMLCollection.h"
-#include "core/html/HTMLFormElement.h"
-#include "core/html/HTMLFrameOwnerElement.h"
-#include "core/html/HTMLHeadElement.h"
-#include "core/html/HTMLInputElement.h"
-#include "core/html/HTMLLinkElement.h"
-#include "core/html/PluginDocument.h"
-#include "core/inspector/InspectorController.h"
-#include "core/inspector/ScriptCallStack.h"
-#include "core/loader/DocumentLoader.h"
-#include "core/loader/FrameLoadRequest.h"
-#include "core/loader/FrameLoader.h"
-#include "core/loader/HistoryItem.h"
-#include "core/loader/SubstituteData.h"
-#include "core/page/Chrome.h"
-#include "core/page/EventHandler.h"
-#include "core/page/FocusController.h"
-#include "core/page/FrameTree.h"
-#include "core/page/Page.h"
-#include "core/page/PrintContext.h"
-#include "core/frame/Settings.h"
-#include "core/rendering/HitTestResult.h"
-#include "core/rendering/RenderBox.h"
-#include "core/rendering/RenderFrame.h"
-#include "core/rendering/RenderLayer.h"
-#include "core/rendering/RenderObject.h"
-#include "core/rendering/RenderTreeAsText.h"
-#include "core/rendering/RenderView.h"
-#include "core/rendering/style/StyleInheritedData.h"
-#include "core/timing/Performance.h"
-#include "core/xml/DocumentXPathEvaluator.h"
-#include "core/xml/XPathResult.h"
-#include "platform/TraceEvent.h"
-#include "platform/UserGestureIndicator.h"
-#include "platform/clipboard/ClipboardUtilities.h"
-#include "platform/fonts/FontCache.h"
-#include "platform/graphics/GraphicsContext.h"
-#include "platform/graphics/GraphicsLayerClient.h"
-#include "platform/graphics/skia/SkiaUtils.h"
-#include "platform/network/ResourceRequest.h"
-#include "platform/scroll/ScrollbarTheme.h"
-#include "platform/scroll/ScrollTypes.h"
-#include "platform/weborigin/KURL.h"
-#include "platform/weborigin/SchemeRegistry.h"
-#include "platform/weborigin/SecurityPolicy.h"
-#include "public/platform/Platform.h"
-#include "public/platform/WebFloatPoint.h"
-#include "public/platform/WebFloatRect.h"
-#include "public/platform/WebLayer.h"
-#include "public/platform/WebPoint.h"
-#include "public/platform/WebRect.h"
-#include "public/platform/WebSize.h"
-#include "public/platform/WebURLError.h"
+#include "platform/Timer.h"
 #include "public/platform/WebVector.h"
 #include "wtf/CurrentTime.h"
-#include "wtf/HashMap.h"
 
 using namespace WebCore;
 
 namespace blink {
 
-static int frameCount = 0;
-
-// Key for a StatsCounter tracking how many WebFrames are active.
-static const char webFrameActiveCount[] = "WebFrameActiveCount";
-
-static void frameContentAsPlainText(size_t maxChars, LocalFrame* frame,
StringBuilder& output)
-{
-    Document* document = frame->document();
-    if (!document)
-        return;
-
-    if (!frame->view())
-        return;
-
-    // TextIterator iterates over the visual representation of the DOM. As
such,
-    // it requires you to do a layout before using it (otherwise it'll crash).
-    document->updateLayout();
-
-    // Select the document body.
-    RefPtr<Range> range(document->createRange());
-    TrackExceptionState exceptionState;
-    range->selectNodeContents(document->body(), exceptionState);
-
-    if (!exceptionState.hadException()) {
-        // The text iterator will walk nodes giving us text. This is similar to
-        // the plainText() function in core/editing/TextIterator.h, but we
implement the maximum
-        // size and also copy the results directly into a wstring, avoiding the
-        // string conversion.
-        for (TextIterator it(range.get()); !it.atEnd(); it.advance()) {
-            it.appendTextToStringBuilder(output, 0, maxChars -
output.length());
-            if (output.length() >= maxChars)
-                return; // Filled up the buffer.
-        }
-    }
-
-    // The separator between frames when the frames are converted to plain
text.
-    const LChar frameSeparator[] = { '\n', '\n' };
-    const size_t frameSeparatorLength = WTF_ARRAY_LENGTH(frameSeparator);
-
-    // Recursively walk the children.
-    const FrameTree& frameTree = frame->tree();
-    for (LocalFrame* curChild = frameTree.firstChild(); curChild; curChild =
curChild->tree().nextSibling()) {
-        // Ignore the text of non-visible frames.
-        RenderView* contentRenderer = curChild->contentRenderer();
-        RenderPart* ownerRenderer = curChild->ownerRenderer();
-        if (!contentRenderer || !contentRenderer->width() ||
!contentRenderer->height()
-            || (contentRenderer->x() + contentRenderer->width() <= 0) ||
(contentRenderer->y() + contentRenderer->height() <= 0)
-            || (ownerRenderer && ownerRenderer->style() &&
ownerRenderer->style()->visibility() != VISIBLE)) {
-            continue;
-        }
-
-        // Make sure the frame separator won't fill up the buffer, and give up
if
-        // it will. The danger is if the separator will make the buffer longer
than
-        // maxChars. This will cause the computation above:
-        //   maxChars - output->size()
-        // to be a negative number which will crash when the subframe is added.
-        if (output.length() >= maxChars - frameSeparatorLength)
-            return;
-
-        output.append(frameSeparator, frameSeparatorLength);
-        frameContentAsPlainText(maxChars, curChild, output);
-        if (output.length() >= maxChars)
-      …
(message too large)

[Andrey Kraynov, 2014-03-26 17:09:24]

The CQ bit was checked by iceman@yandex-team.ru

[commit-bot: I haz the power, 2014-03-26 17:09:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iceman@yandex-team.ru/181013007/210001

[commit-bot: I haz the power, 2014-03-26 17:22:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-26 17:22:45]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[Andrey Kraynov, 2014-03-28 13:17:35]

The CQ bit was checked by iceman@yandex-team.ru

[commit-bot: I haz the power, 2014-03-28 13:17:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iceman@yandex-team.ru/181013007/230001

[commit-bot: I haz the power, 2014-03-28 13:43:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 13:43:02]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[tkent, 2014-03-28 13:46:14]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2014-03-28 13:46:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iceman@yandex-team.ru/181013007/230001

[commit-bot: I haz the power, 2014-03-28 14:25:47]

Change committed as 170329