Reland "Keep browser alive while fetch keepalive URLLoaders exist" [chromium/src : main]

0 views
Skip to first unread message

Helmut Januschka (Gerrit)

unread,
3:31 AM (2 hours ago) 3:31 AM
to Chromium LUCI CQ, Nicolas Ouellet-Payeur, Ryan Sultanem, Greg Thompson, Ming-Ying Chung, Rakina Zata Amni, android-bu...@system.gserviceaccount.com, Barry Pollard, chromium...@chromium.org, Chromium Metrics Reviews, asvitkine...@chromium.org, loading...@chromium.org
Attention needed from Greg Thompson, Ming-Ying Chung, Nicolas Ouellet-Payeur, Rakina Zata Amni and Ryan Sultanem

Helmut Januschka added 1 comment

Patchset-level comments
File-level comment, Patchset 2 (Latest):
Helmut Januschka . resolved

ready for review! see PS1/PS2 diff for fixes!

Open in Gerrit

Related details

Attention is currently required from:
  • Greg Thompson
  • Ming-Ying Chung
  • Nicolas Ouellet-Payeur
  • Rakina Zata Amni
  • Ryan Sultanem
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: If4fd9e056a7dce471ba1353c3f5b8976e003a306
Gerrit-Change-Number: 8181986
Gerrit-PatchSet: 2
Gerrit-Owner: Helmut Januschka <hel...@januschka.com>
Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
Gerrit-Reviewer: Greg Thompson <g...@chromium.org>
Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>
Gerrit-Reviewer: Ming-Ying Chung <my...@chromium.org>
Gerrit-Reviewer: Nicolas Ouellet-Payeur <nico...@chromium.org>
Gerrit-Reviewer: Rakina Zata Amni <rak...@chromium.org>
Gerrit-Reviewer: Ryan Sultanem <rs...@google.com>
Gerrit-CC: Barry Pollard <barryp...@chromium.org>
Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
Gerrit-Attention: Nicolas Ouellet-Payeur <nico...@chromium.org>
Gerrit-Attention: Ryan Sultanem <rs...@google.com>
Gerrit-Attention: Greg Thompson <g...@chromium.org>
Gerrit-Attention: Ming-Ying Chung <my...@chromium.org>
Gerrit-Attention: Rakina Zata Amni <rak...@chromium.org>
Gerrit-Comment-Date: Fri, 31 Jul 2026 07:31:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Ming-Ying Chung (Gerrit)

unread,
4:37 AM (1 hour ago) 4:37 AM
to Helmut Januschka, Chromium LUCI CQ, Nicolas Ouellet-Payeur, Ryan Sultanem, Greg Thompson, Rakina Zata Amni, android-bu...@system.gserviceaccount.com, Barry Pollard, chromium...@chromium.org, Chromium Metrics Reviews, asvitkine...@chromium.org, loading...@chromium.org
Attention needed from Greg Thompson, Helmut Januschka, Nicolas Ouellet-Payeur, Rakina Zata Amni and Ryan Sultanem

Ming-Ying Chung added 1 comment

File content/browser/loader/keep_alive_url_loader.cc
Line 478, Patchset 2 (Latest): *browser_context_);
Ming-Ying Chung . unresolved

What happened to the previous version? Does it fail because `*storage_partition_->browser_context()` is gone at some point?

Open in Gerrit

Related details

Attention is currently required from:
  • Greg Thompson
  • Helmut Januschka
  • Nicolas Ouellet-Payeur
  • Rakina Zata Amni
  • Ryan Sultanem
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: If4fd9e056a7dce471ba1353c3f5b8976e003a306
      Gerrit-Change-Number: 8181986
      Gerrit-PatchSet: 2
      Gerrit-Owner: Helmut Januschka <hel...@januschka.com>
      Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
      Gerrit-Reviewer: Greg Thompson <g...@chromium.org>
      Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>
      Gerrit-Reviewer: Ming-Ying Chung <my...@chromium.org>
      Gerrit-Reviewer: Nicolas Ouellet-Payeur <nico...@chromium.org>
      Gerrit-Reviewer: Rakina Zata Amni <rak...@chromium.org>
      Gerrit-Reviewer: Ryan Sultanem <rs...@google.com>
      Gerrit-CC: Barry Pollard <barryp...@chromium.org>
      Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
      Gerrit-Attention: Helmut Januschka <hel...@januschka.com>
      Gerrit-Attention: Nicolas Ouellet-Payeur <nico...@chromium.org>
      Gerrit-Attention: Ryan Sultanem <rs...@google.com>
      Gerrit-Attention: Greg Thompson <g...@chromium.org>
      Gerrit-Attention: Rakina Zata Amni <rak...@chromium.org>
      Gerrit-Comment-Date: Fri, 31 Jul 2026 08:37:36 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Greg Thompson (Gerrit)

      unread,
      4:55 AM (1 hour ago) 4:55 AM
      to Helmut Januschka, Chromium LUCI CQ, Nicolas Ouellet-Payeur, Ryan Sultanem, Ming-Ying Chung, Rakina Zata Amni, android-bu...@system.gserviceaccount.com, Barry Pollard, chromium...@chromium.org, Chromium Metrics Reviews, asvitkine...@chromium.org, loading...@chromium.org
      Attention needed from Helmut Januschka, Nicolas Ouellet-Payeur, Rakina Zata Amni and Ryan Sultanem

      Greg Thompson added 1 comment

      File content/browser/loader/keep_alive_url_loader.h
      Line 469, Patchset 2 (Latest): const raw_ptr<BrowserContext> browser_context_;
      Greg Thompson . unresolved

      `raw_ref` since it can't be null

      Open in Gerrit

      Related details

      Attention is currently required from:
      Gerrit-Attention: Rakina Zata Amni <rak...@chromium.org>
      Gerrit-Comment-Date: Fri, 31 Jul 2026 08:55:33 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Helmut Januschka (Gerrit)

      unread,
      5:22 AM (16 minutes ago) 5:22 AM
      to Chromium LUCI CQ, Nicolas Ouellet-Payeur, Ryan Sultanem, Greg Thompson, Ming-Ying Chung, Rakina Zata Amni, android-bu...@system.gserviceaccount.com, Barry Pollard, chromium...@chromium.org, Chromium Metrics Reviews, asvitkine...@chromium.org, loading...@chromium.org
      Attention needed from Greg Thompson, Ming-Ying Chung, Nicolas Ouellet-Payeur, Rakina Zata Amni and Ryan Sultanem

      Helmut Januschka added 2 comments

      File content/browser/loader/keep_alive_url_loader.h
      Line 469, Patchset 2: const raw_ptr<BrowserContext> browser_context_;
      Greg Thompson . resolved

      `raw_ref` since it can't be null

      Helmut Januschka

      Done

      File content/browser/loader/keep_alive_url_loader.cc
      Line 478, Patchset 2: *browser_context_);
      Ming-Ying Chung . resolved

      What happened to the previous version? Does it fail because `*storage_partition_->browser_context()` is gone at some point?

      Helmut Januschka

      Yes. `StoragePartitionImpl::~StoragePartitionImpl()` clears its BrowserContext pointer in the destructor body, and member destruction then tears down `KeepAliveURLLoaderService` and its loaders. The previous loader destructor therefore bound a reference from `nullptr`.

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Greg Thompson
      • Ming-Ying Chung
      • Nicolas Ouellet-Payeur
      • Rakina Zata Amni
      • Ryan Sultanem
        Submit Requirements:
          • requirement satisfiedCode-Coverage
          • requirement is not satisfiedCode-Owners
          • requirement is not satisfiedCode-Review
          • requirement is not satisfiedReview-Enforcement
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: comment
          Gerrit-Project: chromium/src
          Gerrit-Branch: main
          Gerrit-Change-Id: If4fd9e056a7dce471ba1353c3f5b8976e003a306
          Gerrit-Change-Number: 8181986
          Gerrit-PatchSet: 3
          Gerrit-Owner: Helmut Januschka <hel...@januschka.com>
          Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
          Gerrit-Reviewer: Greg Thompson <g...@chromium.org>
          Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>
          Gerrit-Reviewer: Ming-Ying Chung <my...@chromium.org>
          Gerrit-Reviewer: Nicolas Ouellet-Payeur <nico...@chromium.org>
          Gerrit-Reviewer: Rakina Zata Amni <rak...@chromium.org>
          Gerrit-Reviewer: Ryan Sultanem <rs...@google.com>
          Gerrit-CC: Barry Pollard <barryp...@chromium.org>
          Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
          Gerrit-Attention: Nicolas Ouellet-Payeur <nico...@chromium.org>
          Gerrit-Attention: Ryan Sultanem <rs...@google.com>
          Gerrit-Attention: Greg Thompson <g...@chromium.org>
          Gerrit-Attention: Ming-Ying Chung <my...@chromium.org>
          Gerrit-Attention: Rakina Zata Amni <rak...@chromium.org>
          Gerrit-Comment-Date: Fri, 31 Jul 2026 09:22:25 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: No
          Comment-In-Reply-To: Greg Thompson <g...@chromium.org>
          Comment-In-Reply-To: Ming-Ying Chung <my...@chromium.org>
          satisfied_requirement
          unsatisfied_requirement
          open
          diffy
          Reply all
          Reply to author
          Forward
          0 new messages