Keep browser alive while fetch keepalive URLLoaders exist [chromium/src : main]
Helmut Januschka (Gerrit)
Helmut Januschka added 1 comment![]( "Open in Gerrit")
hi @my...@chromium.org i think this solves it, verified it on on all platforms, could you PTAL and re-route if its better fit for other reviewers.
and please let me know if you want me to change anything.
for easier testing here is a sampler with a demo server: https://static.januschka.com/i-408010432/index.html if you want to take a look
Related details
- Ming-Ying Chung
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ming-Ying Chung (Gerrit)
Ming-Ying Chung added 1 comment![]( "Open in Gerrit")
void ChromeContentBrowserClient::OnFetchKeepAliveRequestCreated(The overall idea looks okay. But I roughly remember one of the goal of the fetch keepalive in-browser migration is to get rid of the usage of `ScopedKeepAlive` (or all its callbacks `OnKeepaliveRequest*` above). Unfortunately the code is not cleaned up after it's launched.
Given that new callbacks to increase `num_fetch_keepalive_loaders_` are added, I think we want to make sure that they don't really depend on all other existing callbacks, i.e. we only want to keep the requests alive in extreme case (browser shutdown), not due to fetch keepalive legacy path.
Related details
- Helmut Januschka
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 1 comment![]( "Open in Gerrit")
void ChromeContentBrowserClient::OnFetchKeepAliveRequestCreated(The overall idea looks okay. But I roughly remember one of the goal of the fetch keepalive in-browser migration is to get rid of the usage of `ScopedKeepAlive` (or all its callbacks `OnKeepaliveRequest*` above). Unfortunately the code is not cleaned up after it's launched.
Given that new callbacks to increase `num_fetch_keepalive_loaders_` are added, I think we want to make sure that they don't really depend on all other existing callbacks, i.e. we only want to keep the requests alive in extreme case (browser shutdown), not due to fetch keepalive legacy path.
thanks for the insights! the new `num_fetch_keepalive_loaders_` path is fully decoupled from the legacy `num_keepalive_requests_`/`keepalive_timer_` one.
for the legacy cleanup i can file a tracking bug, WDYT?
Related details
- Ming-Ying Chung
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 1 comment![]( "Open in Gerrit")
ping, please let me know if i should re-assign to someone else, or if you want me to address anything
Ming-Ying Chung (Gerrit)
Ming-Ying Chung voted and added 5 comments![]( "Open in Gerrit")
Votes added by Ming-Ying Chung
| Code-Review | +1 |
5 comments
ping, please let me know if i should re-assign to someone else, or if you want me to address anything
LGTM but you need owner review.
void ChromeContentBrowserClient::OnFetchKeepAliveRequestCreated(Helmut JanuschkaThe overall idea looks okay. But I roughly remember one of the goal of the fetch keepalive in-browser migration is to get rid of the usage of `ScopedKeepAlive` (or all its callbacks `OnKeepaliveRequest*` above). Unfortunately the code is not cleaned up after it's launched.
Given that new callbacks to increase `num_fetch_keepalive_loaders_` are added, I think we want to make sure that they don't really depend on all other existing callbacks, i.e. we only want to keep the requests alive in extreme case (browser shutdown), not due to fetch keepalive legacy path.
thanks for the insights! the new `num_fetch_keepalive_loaders_` path is fully decoupled from the legacy `num_keepalive_requests_`/`keepalive_timer_` one.
for the legacy cleanup i can file a tracking bug, WDYT?
sgtm.
void ChromeContentBrowserClient::OnFetchKeepAliveRequestCreated(Better to have commentes (either here or in the header) explaining why we re-introduce the scoped keepalive usage even with the in-browser migration.
fetch_keepalive_profile_keep_alive_ = ScopedProfileKeepAlive::TryAcquire(
profile, ProfileKeepAliveOrigin::kFetchKeepAlive);How if multiple requests from different profiles exist? It looks like only the last profile will be held.
// Tests for https://crbug.com/408010432.We should have proper test description in addition to links to bug.
Related details
- Helmut Januschka
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 3 comments![]( "Open in Gerrit")
void ChromeContentBrowserClient::OnFetchKeepAliveRequestCreated(Helmut JanuschkaBetter to have commentes (either here or in the header) explaining why we re-introduce the scoped keepalive usage even with the in-browser migration.
Done
fetch_keepalive_profile_keep_alive_ = ScopedProfileKeepAlive::TryAcquire(
profile, ProfileKeepAliveOrigin::kFetchKeepAlive);How if multiple requests from different profiles exist? It looks like only the last profile will be held.
gosh, good catch, now tracking loaders per BrowserContext: OnFetchKeepAliveRequestDestroyed() receives the BrowserContext too, and a ScopedProfileKeepAlive is held for every profile with in-flight loaders (acquired on that profile's first loader, released on its last). The pointer at destroy time may belong to a context mid-destruction, so it is only used as a lookup key and never dereferenced. Added a multi-profile browser test (HoldsEveryProfileWithInFlightLoaders) covering this.
We should have proper test description in addition to links to bug.
- Nicolas Ouellet-Payeur
- Rakina Zata Amni
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rakina Zata Amni (Gerrit)
Rakina Zata Amni voted Code-Review+1![]( "Open in Gerrit")
- Helmut Januschka
- Nicolas Ouellet-Payeur
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka removed Mohamed Amir Yosef from this change![]( "Open in Gerrit")
- Alex Ilin
- Greg Thompson
- Kenichi Ishibashi
- Nicolas Ouellet-Payeur
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka removed Alex Ilin from this change![]( "Open in Gerrit")
- Greg Thompson
- Kenichi Ishibashi
- Nicolas Ouellet-Payeur
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka removed Kenichi Ishibashi from this change![]( "Open in Gerrit")
- Greg Thompson
- Nicolas Ouellet-Payeur
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolas Ouellet-Payeur (Gerrit)
Nicolas Ouellet-Payeur voted and added 6 comments![]( "Open in Gerrit")
Votes added by Nicolas Ouellet-Payeur
| Code-Review | +1 |
6 comments
uint64_t num_fetch_keepalive_loaders_ = 0;
std::unique_ptr<ScopedKeepAlive> fetch_keepalive_request_keep_alive_;
// Tracks in-flight fetch keepalive loaders per BrowserContext, holding a
// ScopedProfileKeepAlive for the corresponding original profile while any
// of its loaders exist. Every such profile needs its own hold: destroying
// a profile (e.g. with features::kDestroyProfileOnBrowserClose) tears down
// its StoragePartition and aborts that profile's in-flight loaders. Keys
// are only used as identifiers and are never dereferenced on destruction;
// entries are erased when their loader count drops to zero, which happens
// no later than the corresponding context's destruction.
struct FetchKeepAliveProfileHold {
uint64_t num_loaders = 0;
std::unique_ptr<ScopedProfileKeepAlive> profile_keep_alive;
};
std::map<raw_ptr<content::BrowserContext>, FetchKeepAliveProfileHold>
fetch_keepalive_profile_holds_;
base::TimeTicks fetch_keepalive_hold_started_;
uint64_t fetch_keepalive_max_concurrent_ = 0;IMO this is quite a lot of things to store inline... is ChromeContentBrowserClient really the best place for these? Or could this be some kind of helper class?
// Tracks in-flight fetch keepalive loaders per BrowserContext, holding a
// ScopedProfileKeepAlive for the corresponding original profile while any
// of its loaders exist. Every such profile needs its own hold: destroying
// a profile (e.g. with features::kDestroyProfileOnBrowserClose) tears down
// its StoragePartition and aborts that profile's in-flight loaders. Keys
// are only used as identifiers and are never dereferenced on destruction;
// entries are erased when their loader count drops to zero, which happens
// no later than the corresponding context's destruction.I think this is the same as the comments in `.cc`? Consider deduping this
if (num_fetch_keepalive_loaders_ == 1 &&
!fetch_keepalive_request_keep_alive_ &&Seems overly defensive: I think(?) if `num_fetch_keepalive_loaders_ == 1` then `!fetch_keepalive_request_keep_alive_` is true, and vice-versa. Maybe one of these could be a CHECK?
fetch_keepalive_profile_holds_[browser_context];I guess this can still create 2 keepalives per profile?
- `fetch_keepalive_profile_holds_[original]`
- `fetch_keepalive_profile_holds_[incognito]`
Not that this *breaks* anything(?), it's just a little hacky
if (num_fetch_keepalive_loaders_ == 0) {
return;
}how can this happen? should it be a CHECK?
Related details
- Greg Thompson
- Helmut Januschka
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 5 comments![]( "Open in Gerrit")
uint64_t num_fetch_keepalive_loaders_ = 0;
std::unique_ptr<ScopedKeepAlive> fetch_keepalive_request_keep_alive_;
// Tracks in-flight fetch keepalive loaders per BrowserContext, holding a
// ScopedProfileKeepAlive for the corresponding original profile while any
// of its loaders exist. Every such profile needs its own hold: destroying
// a profile (e.g. with features::kDestroyProfileOnBrowserClose) tears down
// its StoragePartition and aborts that profile's in-flight loaders. Keys
// are only used as identifiers and are never dereferenced on destruction;
// entries are erased when their loader count drops to zero, which happens
// no later than the corresponding context's destruction.
struct FetchKeepAliveProfileHold {
uint64_t num_loaders = 0;
std::unique_ptr<ScopedProfileKeepAlive> profile_keep_alive;
};
std::map<raw_ptr<content::BrowserContext>, FetchKeepAliveProfileHold>
fetch_keepalive_profile_holds_;
base::TimeTicks fetch_keepalive_hold_started_;
uint64_t fetch_keepalive_max_concurrent_ = 0;IMO this is quite a lot of things to store inline... is ChromeContentBrowserClient really the best place for these? Or could this be some kind of helper class?
pulled this out into a dedicated `FetchKeepAliveProcessManager` under `chrome/browser/loader/`
// Tracks in-flight fetch keepalive loaders per BrowserContext, holding a
// ScopedProfileKeepAlive for the corresponding original profile while any
// of its loaders exist. Every such profile needs its own hold: destroying
// a profile (e.g. with features::kDestroyProfileOnBrowserClose) tears down
// its StoragePartition and aborts that profile's in-flight loaders. Keys
// are only used as identifiers and are never dereferenced on destruction;
// entries are erased when their loader count drops to zero, which happens
// no later than the corresponding context's destruction.I think this is the same as the comments in `.cc`? Consider deduping this
Done
if (num_fetch_keepalive_loaders_ == 1 &&
!fetch_keepalive_request_keep_alive_ &&Seems overly defensive: I think(?) if `num_fetch_keepalive_loaders_ == 1` then `!fetch_keepalive_request_keep_alive_` is true, and vice-versa. Maybe one of these could be a CHECK?
Done. Dropped the redundant `!process_keep_alive_` and replaced it with a `CHECK(!process_keep_alive_)`
fetch_keepalive_profile_holds_[browser_context];I guess this can still create 2 keepalives per profile?
- `fetch_keepalive_profile_holds_[original]`
- `fetch_keepalive_profile_holds_[incognito]`
Not that this *breaks* anything(?), it's just a little hacky
yeah that's right, original + OTR both with loaders = two keepalives on the same original profile, but harmles, both pin it, and each drops when its own context's loaders drain.
it's keyed by BrowserContext on purpose, in OnRequestDestroyed the context may already be mid-teardown, so we can only use the raw ptr as a lookup key and can't deref it to find the original.
if (num_fetch_keepalive_loaders_ == 0) {
return;
}how can this happen? should it be a CHECK?
It can't, made it a CHECK. OnFetchKeepAliveRequestCreated/Destroyed are called from KeepAliveURLLoader's constructor/destructor, so they're strictly 1:1 per loader
Related details
- Greg Thompson
- Ming-Ying Chung
- Nicolas Ouellet-Payeur
- Rakina Zata Amni
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rakina Zata Amni (Gerrit)
Rakina Zata Amni voted Code-Review+1![]( "Open in Gerrit")
- Greg Thompson
- Helmut Januschka
- Ming-Ying Chung
- Nicolas Ouellet-Payeur
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolas Ouellet-Payeur (Gerrit)
Nicolas Ouellet-Payeur voted and added 2 comments![]( "Open in Gerrit")
Votes added by Nicolas Ouellet-Payeur
| Code-Review | +1 |
2 comments
!fetch_keepalive_process_manager_) {nit: CHECK instead. `OnFetchKeepAliveRequestCreated` is always called first, right?
fetch_keepalive_profile_holds_[browser_context];Helmut JanuschkaI guess this can still create 2 keepalives per profile?
- `fetch_keepalive_profile_holds_[original]`
- `fetch_keepalive_profile_holds_[incognito]`
Not that this *breaks* anything(?), it's just a little hacky
yeah that's right, original + OTR both with loaders = two keepalives on the same original profile, but harmles, both pin it, and each drops when its own context's loaders drain.
it's keyed by BrowserContext on purpose, in OnRequestDestroyed the context may already be mid-teardown, so we can only use the raw ptr as a lookup key and can't deref it to find the original.
Acknowledged
Related details
- Greg Thompson
- Helmut Januschka
- Ming-Ying Chung
Helmut Januschka (Gerrit)
Helmut Januschka added 1 comment![]( "Open in Gerrit")
nit: CHECK instead. `OnFetchKeepAliveRequestCreated` is always called first, right?
- Greg Thompson
- Ming-Ying Chung
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ming-Ying Chung (Gerrit)
Ming-Ying Chung voted Code-Review+1![]( "Open in Gerrit")
- Greg Thompson
- Helmut Januschka
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Greg Thompson (Gerrit)
Greg Thompson added 10 comments![]( "Open in Gerrit")
BASE_DECLARE_FEATURE(kKeepAliveBrowserProcessAlive);the comment on line 25 says not to add to this file. how about introducing a `chrome/browser/loader/features.{cc,h}` pair for this?
#include "chrome/browser/loader/fetch_keepalive_process_manager.h"move down into the `#if !BUILDFLAG(IS_ANDROID)` block below
std::map<raw_ptr<content::BrowserContext>, ProfileHold> profile_holds_;use `absl::flat_hash_map` as per https://chromium.googlesource.com/chromium/src/+/main/base/containers/README.md#map-and-set-selection-usage-advice
void OnRequestCreated(content::BrowserContext* browser_context);since we're in `//chrome`, wdyt about using `Profile` here rather than `BrowserContext`? could that work, or would we run into trouble during the shutdown/destroyed case?
#include <cstdint>please use `<stdint.h>` since we say `uint64_t` rather than `std::uint64_t`.
if (browser_context) {how will this ever be nullptr? as per comment in content/public/browser/content_browser_client.h, pass `browser_context` by non-const ref if it cannot be null.
if (!hold_started_.is_null()) {how could this ever not be true?
virtual void OnFetchKeepAliveRequestCreated(BrowserContext* browser_context);will this ever be nullptr? if not, pass by non-const reference rather than pointer as per https://google.github.io/styleguide/cppguide.html#Inputs_and_Outputs
Related details
- Helmut Januschka
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 10 comments![]( "Open in Gerrit")
the comment on line 25 says not to add to this file. how about introducing a `chrome/browser/loader/features.{cc,h}` pair for this?
sorry missed that comment! done, moved it to loader/features
#include "base/memory/raw_ptr.h"Helmut Januschkaunused
Done
#include "chrome/browser/loader/fetch_keepalive_process_manager.h"move down into the `#if !BUILDFLAG(IS_ANDROID)` block below
Done
std::map<raw_ptr<content::BrowserContext>, ProfileHold> profile_holds_;use `absl::flat_hash_map` as per https://chromium.googlesource.com/chromium/src/+/main/base/containers/README.md#map-and-set-selection-usage-advice
Done
void OnRequestCreated(content::BrowserContext* browser_context);since we're in `//chrome`, wdyt about using `Profile` here rather than `BrowserContext`? could that work, or would we run into trouble during the shutdown/destroyed case?
Switched the manager API and tracking map to `Profile*`. The destroyed path converts the still-live `BrowserContext` to `Profile*` at the callback boundary.
please use `<stdint.h>` since we say `uint64_t` rather than `std::uint64_t`.
Done
how will this ever be nullptr? as per comment in content/public/browser/content_browser_client.h, pass `browser_context` by non-const ref if it cannot be null.
Done
if (!hold_started_.is_null()) {how could this ever not be true?
this can happen in the `IsShuttingDown()` path, creating a `ScopedKeepAlive` would CHECK, but pending work can still create a loader during teardown.
virtual void OnFetchKeepAliveRequestCreated(BrowserContext* browser_context);will this ever be nullptr? if not, pass by non-const reference rather than pointer as per https://google.github.io/styleguide/cppguide.html#Inputs_and_Outputs
Good point. `StoragePartition::browser_context()` is non-null, changed both callbacks to take non-const references.
Related details
- Greg Thompson
- Ming-Ying Chung
- Nicolas Ouellet-Payeur
- Rakina Zata Amni
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Greg Thompson (Gerrit)
Greg Thompson voted and added 2 comments![]( "Open in Gerrit")
Votes added by Greg Thompson
| Code-Review | +1 |
2 comments
class FetchKeepAliveProcessManager;wrap this in `#if !BUILDFLAG(IS_ANDROID)`.
(sorry i missed this earlier)
#include "chrome/browser/loader/features.h"this also goes in the `!IS_ANDROID` block below
Related details
- Helmut Januschka
- Ming-Ying Chung
- Nicolas Ouellet-Payeur
- Rakina Zata Amni
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolas Ouellet-Payeur (Gerrit)
Nicolas Ouellet-Payeur voted Code-Review+1![]( "Open in Gerrit")
- Helmut Januschka
- Ming-Ying Chung
- Rakina Zata Amni
Helmut Januschka (Gerrit)
Helmut Januschka added 3 comments![]( "Open in Gerrit")
wrap this in `#if !BUILDFLAG(IS_ANDROID)`.
(sorry i missed this earlier)
Done
this also goes in the `!IS_ANDROID` block below
Done
virtual void OnFetchKeepAliveRequestCreated(BrowserContext* browser_context);Helmut Januschkawill this ever be nullptr? if not, pass by non-const reference rather than pointer as per https://google.github.io/styleguide/cppguide.html#Inputs_and_Outputs
Good point. `StoragePartition::browser_context()` is non-null, changed both callbacks to take non-const references.
- Ming-Ying Chung
- Rakina Zata Amni
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rakina Zata Amni (Gerrit)
Rakina Zata Amni voted Code-Review+1![]( "Open in Gerrit")
- Helmut Januschka
- Ming-Ying Chung
Helmut Januschka (Gerrit)
Helmut Januschka added 2 comments![]( "Open in Gerrit")
void OnRequestCreated(content::BrowserContext* browser_context);since we're in `//chrome`, wdyt about using `Profile` here rather than `BrowserContext`? could that work, or would we run into trouble during the shutdown/destroyed case?
Switched the manager API and tracking map to `Profile*`. The destroyed path converts the still-live `BrowserContext` to `Profile*` at the callback boundary.
Done
if (!hold_started_.is_null()) {Helmut Januschkahow could this ever not be true?
this can happen in the `IsShuttingDown()` path, creating a `ScopedKeepAlive` would CHECK, but pending work can still create a loader during teardown.
- Ming-Ying Chung
- Ryan Sultanem
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka voted Auto-Submit+1![]( "Open in Gerrit")
| Auto-Submit | +1 |
Ming-Ying Chung (Gerrit)
Ming-Ying Chung voted Code-Review+1![]( "Open in Gerrit")
- Helmut Januschka
- Ryan Sultanem
Ryan Sultanem (Gerrit)
Ryan Sultanem voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ryan Sultanem
| Code-Review | +1 |
| Commit-Queue | +2 |
1 comment
LGTM for `chrome/browser/profiles/keep_alive/profile_keep_alive_types.*` and `profile/enums.xml`
Related details
- Helmut Januschka
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka voted Commit-Queue+2![]( "Open in Gerrit")
- Ryan Sultanem
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
11 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: chrome/browser/loader/keep_alive_browser_process_alive_browsertest.cc
Insertions: 1, Deletions: 1.
@@ -83,7 +83,7 @@
IN_PROC_BROWSER_TEST_F(FetchKeepAliveProcessAliveBrowserTest,
HoldsEveryProfileWithInFlightLoaders) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
- Profile* first_profile = browser()->profile();
+ Profile* first_profile = browser()->GetProfile();
Profile& second_profile = profiles::testing::CreateProfileSync(
profile_manager,
profile_manager->user_data_dir().AppendASCII("Profile 2"));
```
Change information
Keep browser alive while fetch keepalive URLLoaders exist
Holds a ScopedKeepAlive while any fetch keepalive URLLoader (including
pending fetchLater) exists, gated by the default-off
kKeepAliveBrowserProcessAlive feature, so closing the last window does
not drop pending requests.
- M chrome/browser/chrome_content_browser_client.cc
- M chrome/browser/chrome_content_browser_client.h
- M chrome/browser/loader/BUILD.gn
- A chrome/browser/loader/features.cc
- A chrome/browser/loader/features.h
- A chrome/browser/loader/fetch_keepalive_process_manager.cc
- A chrome/browser/loader/fetch_keepalive_process_manager.h
- A chrome/browser/loader/keep_alive_browser_process_alive_browsertest.cc
- M chrome/browser/profiles/keep_alive/profile_keep_alive_types.cc
- M chrome/browser/profiles/keep_alive/profile_keep_alive_types.h
- M components/keep_alive_registry/keep_alive_types.cc
- M components/keep_alive_registry/keep_alive_types.h
- M content/browser/loader/keep_alive_url_loader.cc
- M content/public/browser/content_browser_client.cc
- M content/public/browser/content_browser_client.h
- M tools/metrics/histograms/metadata/chrome/histograms.xml
- M tools/metrics/histograms/metadata/profile/enums.xml
Code-Review: +1 by Rakina Zata Amni, +1 by Ming-Ying Chung, +1 by Greg Thompson, +1 by Nicolas Ouellet-Payeur, +1 by Ryan Sultanem
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shunya Shishido (Gerrit)
Shunya Shishido has created a revert of this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |