New issue
Advanced search Search tips

Issue 704431 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Dec 7
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Task

Blocking:
issue 655916



Sign in to add a comment

Simplify WebCachePolicy / FrameLoadType determination logic

Project Member Reported by toyoshim@chromium.org, Mar 23 2017

Issue description

The main logic for WebCachePolicy is in FrameFetchContext::determineWebCachePolicy() now.
It still contains some errors, and some conditions are not handled.

Also, that logic is based on FrameLoadType, and FrameLoadType determination logics are distributed here and there.
E.g., browser decides FrameLoadType first, WebLocalFrame also modifies it for history navigations, and FrameLoader determines FrameLoadType again from FrameLoadRequest, etc.

Even after the FrameFetchContext determines the WebCachePolicy, also ResourceFetcher does additional checks in determineRevalidationPolicy, etc...

This bug is a fork of the  issue 602900 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 30 2017

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

commit 4faad2d582a5ee0ed6381d0139bc5333407769d3
Author: toyoshim <toyoshim@chromium.org>
Date: Thu Mar 30 04:19:30 2017

FrameFetchContext should respect BypassingCache for main resources

Existing code respects a parent frame's policy rather than
FrameLoadTypeReloadBypassingCache for main resources.

This isn't consistent to other sub-resources, and it can respect
BypassingCache rather than checking the parent policy because
the parent always return BypassingCache in such cases.

The parent frame can have a different policy if we have an UI
to trigger a per-frame bypassing reload. But we do not and won't today.
Even we have a per-frame bypassing reload, this change should be still
reasonable.

BUG= 704431 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/4faad2d582a5ee0ed6381d0139bc5333407769d3/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/4faad2d582a5ee0ed6381d0139bc5333407769d3/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 30 2017

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

commit d6744241b196c16f0946a00301aebeb7672d4166
Author: toyoshim <toyoshim@chromium.org>
Date: Thu Mar 30 10:34:46 2017

FrameFetchContext should respect BypassingCache

Existing logic in determineWebCachePolicy() respects request conditions
such as request being a conditional, and requesting method being POST,
even for FrameLoadTypeReloadBypassingCache.

But, BypassingCache is stronger condition for cache handling, and should
overwrite ValidatingCacheData.

BUG= 704431 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/d6744241b196c16f0946a00301aebeb7672d4166/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/d6744241b196c16f0946a00301aebeb7672d4166/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 4 2017

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

commit c6f5a1533c9beb6bf91b01bcd998f810c0ec98ce
Author: toyoshim <toyoshim@chromium.org>
Date: Tue Apr 04 12:15:49 2017

FrameFetchContext should not always respect conditional requests

Existing logic respects conditional requests for sub-resources always.
But we should respect WebCachePolicy for the frame.

This old code might affect history navigation performance badly because
it validates cache entries for conditional resources on history navigation.

BUG= 704431 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/c6f5a1533c9beb6bf91b01bcd998f810c0ec98ce/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/c6f5a1533c9beb6bf91b01bcd998f810c0ec98ce/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 11 2017

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

commit f2d2a76a59caaf3065c36a959ce0fa7b7ad42280
Author: toyoshim <toyoshim@chromium.org>
Date: Tue Apr 11 08:49:17 2017

FrameFetchContext should handle all FrameLoadTypes

Now, determineWebCachePolicy() does not care for FrameLoadTypes
that are used in edge cases.

This patch adopts the corresponding logic for such types.

BUG= 704431 

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

[modify] https://crrev.com/f2d2a76a59caaf3065c36a959ce0fa7b7ad42280/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/f2d2a76a59caaf3065c36a959ce0fa7b7ad42280/third_party/WebKit/public/web/WebFrameLoadType.h

Status: Archived (was: Started)

Sign in to add a comment