[Webkit-unassigned] [Bug 158466] Add event support for link preload.

[bugzilla-daemon at webkit.org]

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

--- Comment #39 from Yoav Weiss <yoav at yoav.ws> ---
Comment on attachment 280799
  --> https://bugs.webkit.org/attachment.cgi?id=280799
Patch

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

>> Source/WebCore/loader/LinkPreloadResourceClients.cpp:2
>> + * Copyright 2016 The Chromium Authors. All rights reserved.
> 
> Did the chromium authors write this?  Is there a corresponding crbug?

The crbug is https://bugs.chromium.org/p/chromium/issues/detail?id=552289

I added these files to the Chromium project a few months back. I consulted other Chromium developers for the design.

>> Source/WebCore/loader/LinkPreloadResourceClients.h:28
>> +#define LinkPreloadResourceClients_h
> 
> #pragma once

will change

>> Source/WebCore/loader/LinkPreloadResourceClients.h:81
>> +        return new LinkPreloadScriptResourceClient(loader, resource);
> 
> This is a memory leak.  There's no corresponding delete.  Instead of Optional, I think we should just use std::unique_ptr

Thanks for catching this. I'll change it to use unique_ptr, and add a LinkLoader member that keeps that reference around.

>> Source/WebCore/loader/cache/CachedResourceLoader.cpp:1035
>>  #if !PLATFORM(IOS)
> 
> I think we should consider making iOS loading work more like everything else.  Probably not in this patch, though.

I agree. I think we're using the OS here as a signal for network conditions, where we could most probably address those directly.

>> Source/WebCore/platform/network/ResourceRequestBase.h:152
>> +    void setIgnoreForRequestCounting(bool ignoreForRequestCounting) { m_ignoreForRequestCounting = ignoreForRequestCounting; }
> 
> shouldIgnoreForeRequestCounting and setShouldIgnoreForRequestCounting might be better names.  Or maybe ignoreForRequestCount to match CachedResource.

will change to ignoreForRequestCount for better consistency.

-- 
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/20160609/68c5d626/attachment.html>