I thought we could make it three state, but after doing a code search, I noticed there are condition checks for all four states. I'm not familiar with preloads, and need helps from preload experts.

Yoav is the expert on link preloads, WDYT?

I would think LinkPreload implies Preload in all cases.

m_forPreload is set on speculative preloads, triggered by the {HTML,CSS}PreloadScanner.
m_linkPreload is set on <link rel=preload> preloads.

And yes, <link rel=preload> requests that are triggered from the HTMLPreloadScanner would have both flags turned on.

So if we're interested in using an enum instead, we'd have to know current state when setting either.

What's the motivation for that move? If it's clarity, we can probably change forPreload to make its name describe its meaning better. If the goal is memory savings, a bitwise flag might be simpler to implement.

Looking at the code again, it seems like forPreload is set on all preloads, speculative or link based... However, I'm not sure that's the right behavior, and can try to clean it up.

Re #c4
One clear reason of this is just because using bool is not encouraged. In my mentioned CL, I added one method at a general place, and using bool argument for link flag is not readable there.

IMO, downsides of using bool are
- adding one bool flag makes number of states doubled. developer often overlooks edge states that are under combinations of flags and rarely happen.
- it's unclear which flags are related each other, and could be set together, like this case (no one know the real possible state numbers if each people add each flag...)

Re #c5
If three-state fulfill our requirements, that would be nice. Probably, we may prefer more descriptive name for the speculative preload, for instance { NoPreload, SpeculativePreload, LinkPreload }.

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

commit 58831855e1f0366cf7a9eae129c6d10b711d3195
Author: yoav <yoav@yoav.ws>
Date: Tue Feb 07 11:05:32 2017

Refactor the forPreload to mean speculative preload.

Currently the forPreload() flag means both speculative preload as well
as link preload. as the linkPreload flag indicates link preload, the
forPreload flag is confusing.
This CL renames the flag to speculativePreload and makes sure that code
that currently uses it uses the appropriate signal: link, speculative or
both.

BUG= 675883 

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

[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/core/html/parser/PreloadRequest.cpp
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/core/loader/FrameFetchContext.h
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/core/loader/LinkLoader.cpp
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/core/loader/PingLoader.cpp
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/core/loader/resource/ImageResource.cpp
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/platform/loader/fetch/FetchContext.cpp
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/platform/loader/fetch/FetchContext.h
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/platform/loader/fetch/FetchRequest.cpp
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/platform/loader/fetch/FetchRequest.h
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/platform/loader/fetch/MockFetchContext.h
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.h
[modify] https://crrev.com/58831855e1f0366cf7a9eae129c6d10b711d3195/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Seems the major concern was already resolved. Let me close this with a mechanical CL that removes the TODO for this bug entry.

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

commit 1562bcc16a626de90b19f70d7fcf499e9c2c8de3
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Tue Feb 20 07:56:02 2018

Platform/loader: Remove a stale TODO

We are not exposing |forPreload| boolean flag to FrameFetchContext
any more. So, the major concern for the TODO has already been resolved.

Bug:  675883 
Change-Id: I56c33ab41dac89185a679250f71fe6a8a48d3e69
Reviewed-on: https://chromium-review.googlesource.com/923771
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537734}
[modify] https://crrev.com/1562bcc16a626de90b19f70d7fcf499e9c2c8de3/third_party/WebKit/Source/platform/loader/fetch/FetchParameters.h