[webkit-reviews] review granted: [Bug 55571] [Refactoring] Auto correction panel should be handled by its own class. : [Attachment 88745] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted MORITA Hajime <morrita at google.com>'s
request for review:
Bug 55571: [Refactoring] Auto correction panel should be handled by its own
class.
https://bugs.webkit.org/show_bug.cgi?id=55571

Attachment 88745: Patch
https://bugs.webkit.org/attachment.cgi?id=88745&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=88745&action=review

> Source/WebCore/editing/Editor.cpp:-3546
>	   bool shouldCheckSpellingAndGrammar = true;
> -#if SUPPORT_AUTOCORRECTION_PANEL
>	   // Don't check spelling and grammar if the change of selection is
triggered by spelling correction itself.
>	   shouldCheckSpellingAndGrammar = !(options &
SelectionController::SpellCorrectionTriggered);
> -#endif

Since we are removing this #if, we should combine the definition and
initialization into a single line.

> Source/WebCore/editing/SpellingCorrectionController.h:77
> +#define UNLESS_ENABLED(functionBody) functionBody

I think you should just put the braces here in the macro so we can use ( )
instead of ({ }) around all those function bodies.

> Source/WebCore/editing/SpellingCorrectionController.h:80
> +class SpellingCorrectionController {

This class should be marked noncopyable.

> Source/WebCore/editing/SpellingCorrectionController.h:83
> +    SpellingCorrectionController(Frame*) UNLESS_ENABLED({})
> +    ~SpellingCorrectionController() UNLESS_ENABLED({})

This technique with all the macros on each function seems a little ugly to me,
but I have no better suggestion.