michaeln@, can you give some guidance here?

I guess I'd first like to add a UseCounter for pages that:
1. Do not have a manifest attr on the html element
2. Still do not want to issue preloads before the appcache is enabled

This is low priority because I don't believe this will speed up things very much.

I'm not familiar with what it is that the preload scanner is waiting for or why?

        if (match(tagImpl, htmlTag) && token.getAttributeItem(manifestAttr)) {
            m_isAppCacheEnabled = true;
            return;
        }

This test probably isn't accomplishing what the author expected it to.

Pages that do not have a manifst attribute can be associated with an appcache (main + sub resources are read from the associated appcache). And the first time a page that has a manifest attribute is loaded, all the resources are retrieved over the network. Also if the page was vended by a serviceworker, the manifest attribute is ignored.

What would happen if all logic related to 'if appcache' was deleted from the preload scanner?

So the code snippet you pasted is a quick bailout of the preload scanner if the subresources are likely to be in appcache. There is an additional check in HTMLDocumentParser so that we wait for AppCache to be initialized before sending out any preload requests.

If we removed the quick bailout code in #3, we would just accumulate more preload requests until the documentElement is available, then send them out with an initialized AppCache. I imagine AppCache then intercepts requests in the content layer but I am not sure.

If we removed the waiting code in HTMLDocumentParser, I think we would actually go over the network before appcache is initialized for the first couple of preloads.

This issue is basically asking if we could only use the quick bailout method. Judging from your response, it seems like if the main resource is associated with an appcache but does not have a manifest attr, then it is being served by an appcache. This makes me think that simplifying the logic here is possible, and we can send preload requests from these resources without fear. Do I have this right?

I can't tell you want to happen when an appcache is being used by the document. Also I don't know how the scanner and the parser conspire together to decide what to preload when.

Do you want to skip pre-loading altogether and not do anything at all? This is what the code in the preload scanner seems to suggest.

Do you want to pre-load resources out of the appcache? This is what deferring until the appcache is initialized seems to suggest.

This much i know, the test in the preload scanner is a poor heuristic for "appcache is being used", it can give false positives and false negatives. On that basis alone, i think it can be removed.

Ok thanks this is helpful. I'll try to answer some of your questions. The current logic is as follows:
1. Scanner bails out of sending parser any preloads if it finds a manifest attr.
2. If there is no manifest attr, the parser waits until AppCache is initialized before sending out any preloads the scanner has found.

I think the reasoning fpr (2) was that we don't want to waste bytes downloading something that is in AppCache. For (1), I'm not sure what the reasoning is, as I believe that even if appcache is being used, it might not be used for all resources.

It would really be nice to avoid waiting, but I think at this point we won't actually gain much performance benefit (at least from looking at tracing). I was seeing something like 1ms time between getting the tokens on the main thread and receiving the documentElementAvailable callback.

However, if we remove the manifest attr check I think we would only see a positive performance improvement (by preloading out of appcache or network). I will make that change now.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/91146fe960885eae005f3ac14f29dacc93a98dac

commit 91146fe960885eae005f3ac14f29dacc93a98dac
Author: csharrison <csharrison@chromium.org>
Date: Thu Jul 28 01:19:55 2016

Dont bail out of preload scanning if a appcache manifest is found

The preload scanner bails if it sees an appcache manifest in the markup.
This behavior isn't really correct, as we already wait for appcache to
be initialized before sending out any requests. Also, there could be
resources that still need to be loaded over the network.

BUG= 630717 

Review-Url: https://codereview.chromium.org/2187193002
Cr-Commit-Position: refs/heads/master@{#408304}

[modify] https://crrev.com/91146fe960885eae005f3ac14f29dacc93a98dac/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp
[modify] https://crrev.com/91146fe960885eae005f3ac14f29dacc93a98dac/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.h

(Assigning this to Charles for now since you seem to have been already working & investigating this)

Hm, I want to close this as fixed, actually. All my tracing has found very little to be gained if we start ignoring AppCache. The only way I'd propose moving forward on this is if we can ignore AppCache for a subset of Documents.

michaeln@, is there any sort of reasonable heuristic that will tell us if appcache might be enabled in Blink, before the document element is available?

If not, I don't a few ms is worth breaking AppCache.

Ok-- let's close this for now as it'll unlikely be worth exploring further based on your tracing investigation.  If we start to see this might be potentially causing noticeable delay again we could file a new bug.

Does preload deposit the resource in blink's memory cache?

The strongest heuristic for whether an appcache is in play is whether the main resource was loaded from an appcache. The renderer can tell if it was or wasn't. But the renderer can't tell whether any given subresource will come out of the appcache or be retreived over the network, it can only tell after the resource has been retrieved.

Yes preloading populates the MemoryCache.

Thanks for the insight. Can you give a pointer to how to tell if the main resource was loaded from an appcache from the renderer? Can you tell easily in Blink?

It may be nice as a possible improvement in the future to only delay preloads if the main resource was serviced by AppCache, esp if false positive fetches are low.

I made comment on the other bug about this. The browser side is in a much better position to figure out if its appached or not.

https://bugs.chromium.org/p/chromium/issues/detail?id=632368#c5

It would not make sense to determine if a resource is fresh enough in the http cache and to avoid preloading it from within the renderer. Pretty much same thing for appcache.

Hm, so are you saying that we should just send the requests even before ApplicationCacheHost::selectCache{With,Without}Manifest is called? Sorry I'm very unfamiliar with the appcache, will it be intercepted in the browser process regardless?

re c#12, see ResourceResponse::appCacheID() for a way to tell the resource was read from an appcache, 0 means it didn't come from an appcache.