[Webkit-unassigned] [Bug 165028] Reduce platform ifdefs in WebKit2 custom protocols implementation

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 15 08:40:55 PST 2017


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

--- Comment #10 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #9)
> Comment on attachment 299133 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=299133&action=review

Thanks for the review!

> > Source/WebKit2/UIProcess/Cocoa/CustomProtocolManagerClient.h:46
> > +    void startLoading(CustomProtocolManagerProxy&, uint64_t /*customProtocolID*/, const WebCore::ResourceRequest&) override;
> > +    void stopLoading(CustomProtocolManagerProxy&, uint64_t /*customProtocolID*/) override;
> > +    void invalidate(CustomProtocolManagerProxy&) override;
> 
> WebKit coding style says to use final, not override.

Even for a class already marked as final?

> > Source/WebKit2/UIProcess/Cocoa/CustomProtocolManagerClient.mmSource/WebKit2/UIProcess/Network/CustomProtocols/mac/CustomProtocolManagerProxyMac.mm:143
> > +    // FIXME: iterate the loader map and remove loaders for the given manager.
> 
> Why is it safe to leave this unimplemented?

Because it's a refactoring, this is already unimplemented with current code. I was afraid of breaking something trying to implement this, since I'm not familiar with how custom protocols work in Mac and I don't have a way to test it. So, I think it's better if someone who knows this code can implement this afterwards.

> > Source/WebKit2/UIProcess/WebProcessPool.cpp:371
> > +    for (const auto& scheme : globalURLSchemesWithCustomProtocolHandlers())
> > +        parameters.urlSchemesRegisteredForCustomProtocols.append(scheme);
> > +
> > +    for (const auto& scheme : m_urlSchemesRegisteredForCustomProtocols)
> > +        parameters.urlSchemesRegisteredForCustomProtocols.append(scheme);
> 
> I suggest just auto& rather than const auto&.

Ok.

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


More information about the webkit-unassigned mailing list