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

Issue 464933002: Don't increment lazy keep alive count on redirects. (Closed)

Created:
6 years, 4 months ago by msimonides
Modified:
6 years, 4 months ago
Reviewers:
agl, Matt Perry
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, cbentzel+watch_chromium.org, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Don't increment lazy keep alive count on redirects. The ChromeExtensionNetworkDelegate::OnBeforeURLRequest is called for new requests as well as for redirects. This results in the lazy keep alive count in extensions to be incremented several times if there is a redirect but then it is decremented only ones when the request finishes. In consequence the lazy extension page never shuts down because the count never reaches zero. With this commit requests that have been redirected at least once are ignored when incrementing the keep alive count. BUG=175279 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289260

Patch Set 1 #

Patch Set 2 : Only filter out redirects on REQUEST_START. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -0 lines) Patch
M chrome/browser/net/chrome_extensions_network_delegate.cc View 1 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
msimonides
6 years, 4 months ago (2014-08-12 15:33:25 UTC) #1
agl
No idea about this. Will rubber stamp if mpcomplete agrees with it.
6 years, 4 months ago (2014-08-12 21:30:55 UTC) #2
Matt Perry
LGTM
6 years, 4 months ago (2014-08-12 23:27:27 UTC) #3
agl
lgtm
6 years, 4 months ago (2014-08-12 23:29:15 UTC) #4
msimonides
The CQ bit was checked by msimonides@opera.com
6 years, 4 months ago (2014-08-13 06:34:44 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msimonides@opera.com/464933002/20001
6 years, 4 months ago (2014-08-13 06:36:52 UTC) #6
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_chromium_gn_rel on tryserver.chromium.linux ...
6 years, 4 months ago (2014-08-13 09:12:54 UTC) #7
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-13 09:19:56 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_gn_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_rel/builds/5435)
6 years, 4 months ago (2014-08-13 09:19:57 UTC) #9
msimonides
The CQ bit was checked by msimonides@opera.com
6 years, 4 months ago (2014-08-13 09:36:45 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msimonides@opera.com/464933002/20001
6 years, 4 months ago (2014-08-13 09:38:55 UTC) #11
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_chromium_gn_rel on tryserver.chromium.linux ...
6 years, 4 months ago (2014-08-13 09:44:52 UTC) #12
commit-bot: I haz the power
6 years, 4 months ago (2014-08-13 10:33:28 UTC) #13
Message was sent while issue was closed.
Change committed as 289260

Powered by Google App Engine
This is Rietveld 408576698