Bug 153560 - Cache results of data detection in the UI process when load completes
Summary: Cache results of data detection in the UI process when load completes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Enrica Casucci
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-27 14:40 PST by Enrica Casucci
Modified: 2016-01-27 17:36 PST (History)
5 users (show)

See Also:


Attachments
Patch (27.65 KB, patch)
2016-01-27 16:37 PST, Enrica Casucci
no flags Details | Formatted Diff | Diff
Patch that can be applied (27.76 KB, patch)
2016-01-27 16:44 PST, Enrica Casucci
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrica Casucci 2016-01-27 14:40:16 PST
It is necessary to cached the data detection results as soon as the detection has been performed.
We need to keep them around for the clients to be able to retrieve the full url for data detectors links.
Comment 1 Enrica Casucci 2016-01-27 16:37:55 PST
Created attachment 270061 [details]
Patch
Comment 2 Enrica Casucci 2016-01-27 16:44:43 PST
Created attachment 270064 [details]
Patch that can be applied
Comment 3 WebKit Commit Bot 2016-01-27 16:46:21 PST
Attachment 270064 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Cocoa/WebPageProxyCocoa.mm:29:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/Shared/Cocoa/DataDetectionResult.mm:63:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/Cocoa/DataDetectionResult.mm:63:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 3 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Tim Horton 2016-01-27 16:52:00 PST
Comment on attachment 270064 [details]
Patch that can be applied

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

> Source/WebCore/loader/EmptyClients.h:272
> +    virtual void dispatchDidFinishDataDetection(NSArray *) override { }

It's getting weirder and weirder that this is an NSArray instead of a Vector<RetainPtr<>>, but we'll fix it in a follow up?

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2989
> +    send(Messages::WebPageProxy::SetDataDetectionResult(dataDetectionResult));

Seems like this should have a trailing 's'
Comment 5 Enrica Casucci 2016-01-27 17:09:15 PST
(In reply to comment #4)
> Comment on attachment 270064 [details]
> Patch that can be applied
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=270064&action=review
> 
> > Source/WebCore/loader/EmptyClients.h:272
> > +    virtual void dispatchDidFinishDataDetection(NSArray *) override { }
> 
> It's getting weirder and weirder that this is an NSArray instead of a
> Vector<RetainPtr<>>, but we'll fix it in a follow up?
> 
I keep it as an NSArray because this is how DD returns ii and how it is consumed in the UI process. Using a Vector would require an unnecessary copy.
> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2989
> > +    send(Messages::WebPageProxy::SetDataDetectionResult(dataDetectionResult));
> 
> Seems like this should have a trailing 's'
I've used the singular when I'm passing the struct that contains results (plural).
Comment 6 Enrica Casucci 2016-01-27 17:36:43 PST
Committed revision 195722.