[Webkit-unassigned] [Bug 193881] Use a load optimizer for some sites

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=193881

--- Comment #10 from Jiewen Tan <jiewen_tan at apple.com> ---
Comment on attachment 360281
  --> https://bugs.webkit.org/attachment.cgi?id=360281
Patch

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

Thanks Youenn and Brent for reviewing the patch.

>> Source/WebKit/UIProcess/Cocoa/LoadOptimizer.mm:27
>> +#import "LoadOptimizer.h"
> 
> I would rename LoadOptimizer to NavigationLoadOptimizer since this is navigation load specific.
> Maybe there is a more specific term than Optimizer as well, not sure.

I don't think we should focus too much on the naming.

>> Source/WebKit/UIProcess/Cocoa/NavigationState.mm:472
>> +        auto* localCompletionHandler = new WTF::Function<void (bool)>(WTFMove(completionHandler));
> 
> This preexisting code could be modernized.
> Maybe we should use a BlockPtr?
> Also, if there is a bug in LSAppLink which makes it call the completion handler twice, we would delete twice localCompletionHandler.

You are right. Could we modernize the code in a separate patch since it is unrelated to what I have done here? Bug 193886.

>> Source/WebKit/UIProcess/Cocoa/NavigationState.mm:490
>> +    completionHandler(false);
> 
> This could be written this way:
> #if HAVE(LOAD_OPTIMIZER)
>     page.websiteDataStore().loadOptimizer().tryLoading(navigationAction->request(), page, WTFMove(completionHandler));
> #else
>     completionHandler(false);
> #endif
> That way it is up to the loadOptimizer instance to know when it can or not optimize the load.

Good catch. I would keep that in mind. Since we have this lock step development process for things involving internal. I will address this later.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190127/bf32393e/attachment.html>