Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(274)

Issue 345643003: Http cache: Implement a timeout for the cache lock. (Closed)

Created:
6 years, 6 months ago by rvargas (doing something else)
Modified:
6 years, 6 months ago
Reviewers:
davidben
CC:
chromium-reviews, cbentzel+watch_chromium.org, gavinp+disk_chromium.org, mmenke
Project:
chromium
Visibility:
Public.

Description

Http cache: Implement a timeout for the cache lock. The cache has a single writer / multiple reader lock to avoid downloading the same resource n times. However, it is possible to block many tabs on the same resource, for instance behind an auth dialog. This CL implements a 20 seconds timeout so that the scenario described in the bug results in multiple authentication dialogs (one per blocked tab) so the user can know what to do. It will also help with other cases when the single writer blocks for a long time. The timeout is somewhat arbitrary but it should allow medium size resources to be downloaded before starting another request for the same item. The general solution of detecting progress and allow readers to start before the writer finishes should be implemented on another CL. BUG=6697, 46104 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279326

Patch Set 1 : #

Total comments: 11

Patch Set 2 : Add constant #

Unified diffs Side-by-side diffs Delta from patch set Stats (+101 lines, -2 lines) Patch
M net/base/net_error_list.h View 1 chunk +3 lines, -0 lines 0 comments Download
M net/http/http_cache.h View 2 chunks +7 lines, -0 lines 0 comments Download
M net/http/http_cache.cc View 4 chunks +9 lines, -1 line 0 comments Download
M net/http/http_cache_transaction.h View 3 chunks +9 lines, -0 lines 0 comments Download
M net/http/http_cache_transaction.cc View 1 4 chunks +39 lines, -1 line 0 comments Download
M net/http/http_cache_unittest.cc View 1 chunk +27 lines, -0 lines 0 comments Download
M net/http/mock_http_cache.h View 1 chunk +3 lines, -0 lines 0 comments Download
M net/http/mock_http_cache.cc View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
rvargas (doing something else)
PTAL
6 years, 6 months ago (2014-06-20 01:02:18 UTC) #1
davidben
Any timeout here is going to be arbitrary, but 20 seconds seems really on the ...
6 years, 6 months ago (2014-06-20 19:28:36 UTC) #2
rvargas (doing something else)
Thanks. The main use case is the one of a lot of tabs and some ...
6 years, 6 months ago (2014-06-20 22:40:06 UTC) #3
davidben
LGTM. Well, 20 seconds for an "eventually works" is probably long enough that it doesn't ...
6 years, 6 months ago (2014-06-20 23:08:18 UTC) #4
rvargas (doing something else)
The CQ bit was checked by rvargas@chromium.org
6 years, 6 months ago (2014-06-24 02:11:30 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rvargas@chromium.org/345643003/40001
6 years, 6 months ago (2014-06-24 02:12:30 UTC) #6
commit-bot: I haz the power
6 years, 6 months ago (2014-06-24 05:33:51 UTC) #7
Message was sent while issue was closed.
Change committed as 279326

Powered by Google App Engine
This is Rietveld 408576698