Bug 74033 - Refactoring: SpellChecker::requestCheckingFor should take SpellCheckRequest object
Summary: Refactoring: SpellChecker::requestCheckingFor should take SpellCheckRequest o...
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on:
Blocks: 73616
  Show dependency treegraph
Reported: 2011-12-07 16:15 PST by Shinya Kawanaka
Modified: 2011-12-08 02:46 PST (History)
1 user (show)

See Also:

Patch (11.27 KB, patch)
2011-12-07 21:11 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (11.62 KB, patch)
2011-12-08 00:00 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shinya Kawanaka 2011-12-07 16:15:40 PST
This will make it easy to merge asynchronous path and synchronous path of spellcheck.
Comment 1 Shinya Kawanaka 2011-12-07 21:11:56 PST
Created attachment 118325 [details]
Comment 2 Hajime Morrita 2011-12-07 23:02:38 PST
Comment on attachment 118325 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=118325&action=review

Basically looks fine. let's have one iteration.

> Source/WebCore/editing/SpellChecker.cpp:127
> +    request->setSequence(++m_lastRequestSequence);

Let us make sure that published id never conflict against the magic number. 
I know this will never happen in reality, but preventing it explicitly gives us more confidence.

> Source/WebCore/editing/SpellChecker.h:66
> +    Element* m_rootEditableElement;

Let's make this RefPtr to prevent possible dangling pointer access.

> Source/WebCore/editing/SpellChecker.h:78
> +    static PassRefPtr<SpellCheckRequest> createRequest(TextCheckingTypeMask, PassRefPtr<Range> checkingRange, PassRefPtr<Range> paragraphRange);

SpellCheckerRequest::create() is our standard way if there is no special reason.
Comment 3 Shinya Kawanaka 2011-12-08 00:00:46 PST
Created attachment 118340 [details]
Comment 4 WebKit Review Bot 2011-12-08 02:46:47 PST
Comment on attachment 118340 [details]

Clearing flags on attachment: 118340

Committed r102328: <http://trac.webkit.org/changeset/102328>
Comment 5 WebKit Review Bot 2011-12-08 02:46:51 PST
All reviewed patches have been landed.  Closing bug.