Clear ResourceFetcher::m_validatedURLs if too many entries are added

Clear ResourceFetcher::m_validatedURLs if too many entries are added

This is to avoid memory bloat due to m_validatedURLs.

BUG=52411
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185369

[kouhei (in TOK), 2014-11-14 08:37:50]

kouhei@chromium.org changed reviewers:
+ kouhei@chromium.org

[kouhei (in TOK), 2014-11-14 08:37:51]

https://codereview.chromium.org/723423002/diff/1/Source/core/fetch/ResourceFe...
File Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/723423002/diff/1/Source/core/fetch/ResourceFe...
Source/core/fetch/ResourceFetcher.cpp:698: m_validatedURLs.clear();
Can we have a ring buffer instead?

[hiroshige, 2014-11-14 09:02:05]

https://codereview.chromium.org/723423002/diff/1/Source/core/fetch/ResourceFe...
File Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/723423002/diff/1/Source/core/fetch/ResourceFe...
Source/core/fetch/ResourceFetcher.cpp:698: m_validatedURLs.clear();
On 2014/11/14 08:37:51, kouhei wrote:
> Can we have a ring buffer instead?

Done.

[kouhei (in TOK), 2014-11-14 09:15:58]

https://codereview.chromium.org/723423002/diff/1/Source/core/fetch/ResourceFe...
File Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/723423002/diff/1/Source/core/fetch/ResourceFe...
Source/core/fetch/ResourceFetcher.cpp:698: m_validatedURLs.clear();
On 2014/11/14 09:02:05, hiroshige wrote:
> On 2014/11/14 08:37:51, kouhei wrote:
> > Can we have a ring buffer instead?
> 
> Done.

Sorry, I guess this is a better approach for now. I didn't notice
m_validatedURLs is a HashSet.

s/10000/kMaxValidatedURLsSize/ and add a comment that this is to prevent memory
bloat on m_validatedURLs.

https://codereview.chromium.org/723423002/diff/20001/Source/core/fetch/Resour...
File Source/core/fetch/ResourceFetcher.h (right):

https://codereview.chromium.org/723423002/diff/20001/Source/core/fetch/Resour...
Source/core/fetch/ResourceFetcher.h:209: Deque<String>
m_validatedURLsRingBuffer;
My bad. I didn't notice m_validatedURLs was a HashSet.
If we could use base/, we could use MRUCache here, but no access to base/ is
allowed yet.

[hiroshige, 2014-11-14 09:29:22]

Reverted change in Patch Set 2 and defined kMaxValidatedURLsSize based on Patch
Set 1.

https://codereview.chromium.org/723423002/diff/1/Source/core/fetch/ResourceFe...
File Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/723423002/diff/1/Source/core/fetch/ResourceFe...
Source/core/fetch/ResourceFetcher.cpp:698: m_validatedURLs.clear();
Done in Patch Set 3.

[kouhei (in TOK), 2014-11-14 09:43:50]

lgtm

[hiroshige, 2014-11-14 11:26:27]

The CQ bit was checked by hiroshige@chromium.org

[commit-bot: I haz the power, 2014-11-14 11:26:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/723423002/40001

[commit-bot: I haz the power, 2014-11-14 12:11:16]

Committed patchset #3 (id:40001) as 185369

[PhistucK, 2014-11-14 12:30:36]

phistuck@gmail.com changed reviewers:
+ phistuck@gmail.com

[PhistucK, 2014-11-14 12:30:37]

https://codereview.chromium.org/723423002/diff/40001/Source/core/fetch/Resour...
File Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/723423002/diff/40001/Source/core/fetch/Resour...
Source/core/fetch/ResourceFetcher.cpp:682: static const int
kMaxValidatedURLsSize = 10000;
This seems like a lot to me (10,000 URLs, right?). Imagine the URL is 2,000
characters in length - almost 20MB. I am not sure this bloat (only for validated
URL caching) is worth the benefit. Will 1,000 (or even 5,000) be too little?
This is per renderer (I think?) - 1,000 - 5,000 look sufficiently much to me...