[webkit-reviews] review granted: [Bug 226267] Refactor some data detector code in WebCore::Frame : [Attachment 429763] Patch

[bugzilla-daemon at webkit.org]

Andy Estes <aestes at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 226267: Refactor some data detector code in WebCore::Frame
https://bugs.webkit.org/show_bug.cgi?id=226267

Attachment 429763: Patch

https://bugs.webkit.org/attachment.cgi?id=429763&action=review




--- Comment #2 from Andy Estes <aestes at apple.com> ---
Comment on attachment 429763
  --> https://bugs.webkit.org/attachment.cgi?id=429763
Patch

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

> Source/WebCore/editing/cocoa/DataDetection.h:44
> +class HTMLElement;

Nit: T < i

> Source/WebCore/editing/cocoa/DataDetection.h:49
> +#if ENABLE(IMAGE_EXTRACTION)
> +struct ImageExtractionDataDetectorInfo;
> +#endif

Nit: I think it's preferable to remove the `#if` here since the forward
declaration is harmless even when `ENABLE(IMAGE_EXTRACTION)` is false.

> Source/WebCore/page/Frame.h:104
> +#if ENABLE(DATA_DETECTION)
> +class DataDetectionResultsStorage;
> +#endif

Nit: ditto

> Source/WebCore/page/cocoa/DataDetectionResultsStorage.h:43
> +    DataDetectionResultsStorage() = default;
> +    ~DataDetectionResultsStorage() = default;

Nit: I think the compiler will generate a default constructor and destructor
without you having to declare them.

Note that you also get a default copy constructor. Do you want to
`WTF_MAKE_NONCOPYABLE` this?