New issue
Advanced search Search tips

Issue 891891 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug
Proj-Servicification

Blocked on:
issue 908324

Blocking:
issue 905971
issue 737282
issue 870172



Sign in to add a comment

NetworkContext::SetCorsOriginAccessListsForOrigin doesn't handle network process restarts

Project Member Reported by jam@chromium.org, Oct 3

Issue description

The state that's set in the network process by this call is lost if the network process crashes. Do we need to call this again on restarts?

This may be a non-issue now since it appears this is only called by a browsertest (if so, should the method have a ForTesting suffix?).
 
Components: Internals>Services>Network
Cc: dxie@chromium.org
Cc: yhirano@chromium.org
Thanks you for finding the issue, this will be used to support Chrome Extensions' manifest whitelist very soon. So, yes, I should handle the network service restarts.

At this moment, this code path means only when kOutOfBlinkCORS is enabled, but we will start Canary experiments in a couple of weeks. So, definitely I should fix this ASAP.
Labels: Hotlist-KnownIssue
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f

commit ce4487b7a0d0ef150b5edea19f2db53668dd7d0f
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Tue Oct 23 18:05:53 2018

Make CORB exceptions for plugins resilient to NetworkService crashes.

Bug:  891904 , 891891
Change-Id: I1e05b2527ff7f2e2c452525fa3a460dedc96e9f5
Reviewed-on: https://chromium-review.googlesource.com/c/1263555
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602020}
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/content/browser/network_service_instance.cc
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/content/public/browser/network_service_instance.h
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/content/public/test/ppapi_test_utils.cc
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/content/public/test/ppapi_test_utils.h
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/content/test/BUILD.gn
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/ppapi/BUILD.gn
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/ppapi/shared_impl/ppapi_constants.h
[add] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/ppapi/tests/corb_test_plugin.cc
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/ppapi/tests/test_url_loader.cc
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/ppapi/tests/test_url_loader.h
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/ppapi/tests/test_utils.cc
[modify] https://crrev.com/ce4487b7a0d0ef150b5edea19f2db53668dd7d0f/ppapi/tests/test_utils.h

r602020 from #c5 is not directly related to this bug, but I hope that the helper functions introduced in that CL (i.e. RegisterNetworkServiceCrashHandler) are useful/helpful for somebody truing to solve the OOR-CORS problem.
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 26

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f99066c58a0bfc6e950d877eace8d057b255954

commit 2f99066c58a0bfc6e950d877eace8d057b255954
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Oct 26 20:14:09 2018

Make CORB exceptions for plugins resilient to NetworkService crashes.

TBR=lukasza@chromium.org

(cherry picked from commit ce4487b7a0d0ef150b5edea19f2db53668dd7d0f)

Bug:  891904 , 891891
Change-Id: I1e05b2527ff7f2e2c452525fa3a460dedc96e9f5
Reviewed-on: https://chromium-review.googlesource.com/c/1263555
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602020}
Reviewed-on: https://chromium-review.googlesource.com/c/1302658
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#349}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/content/browser/network_service_instance.cc
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/content/public/browser/network_service_instance.h
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/content/public/test/ppapi_test_utils.cc
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/content/public/test/ppapi_test_utils.h
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/content/test/BUILD.gn
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/ppapi/BUILD.gn
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/ppapi/shared_impl/ppapi_constants.h
[add] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/ppapi/tests/corb_test_plugin.cc
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/ppapi/tests/test_url_loader.cc
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/ppapi/tests/test_url_loader.h
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/ppapi/tests/test_utils.cc
[modify] https://crrev.com/2f99066c58a0bfc6e950d877eace8d057b255954/ppapi/tests/test_utils.h

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/2f99066c58a0bfc6e950d877eace8d057b255954

Commit: 2f99066c58a0bfc6e950d877eace8d057b255954
Author: lukasza@chromium.org
Commiter: lukasza@chromium.org
Date: 2018-10-26 20:14:09 +0000 UTC

Make CORB exceptions for plugins resilient to NetworkService crashes.

TBR=lukasza@chromium.org

(cherry picked from commit ce4487b7a0d0ef150b5edea19f2db53668dd7d0f)

Bug:  891904 , 891891
Change-Id: I1e05b2527ff7f2e2c452525fa3a460dedc96e9f5
Reviewed-on: https://chromium-review.googlesource.com/c/1263555
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602020}
Reviewed-on: https://chromium-review.googlesource.com/c/1302658
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#349}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Blockedon: 908324
Blocking: 905971
Labels: OOR-CORS
Status: Started (was: Assigned)
Now I'm going to make a change to add cors_origin_access_list to the NetworkContextParams so that CreateNetworkContext call can take it to restore the existing per-profile access list.

https://chromium-review.googlesource.com/c/chromium/src/+/1351208/3/chrome/browser/net/profile_network_context_service.cc

Shall we do something additional to make it used on network process restarts?
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8893ec99d0180595cf950a2f7bad2869d9c394b7

commit 8893ec99d0180595cf950a2f7bad2869d9c394b7
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Fri Nov 30 04:58:52 2018

OOR-CORS: Manage per-profile access list even for NetworkService

This patch makes BrowserContext manages per-profile CORS access lists
even if NetworkService is enabled, and use it to setup initial access
lists for the non-primary NetworkContext. It will also work for
restoring per-profile CORS settings on network service restarts.

This patch makes following tests work even with
--enable-features=OutOfBlinkCors,NetworkService.

- CrossOriginReadBlockingExtensionTest.ProgrammaticContentScriptVsAppCache
- CrossOriginReadBlockingExtensionTest.WebViewContentScript
- ExtensionWebRequestApiTest.ExtensionRequests
- PlatformAppBrowserTest.Isolation

Bug:  908324 , 891891
Change-Id: Ib0cfc2f5633f25187366a4d7d63168d60ea51f71
Reviewed-on: https://chromium-review.googlesource.com/c/1351208
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612567}
[modify] https://crrev.com/8893ec99d0180595cf950a2f7bad2869d9c394b7/chrome/browser/net/profile_network_context_service.cc
[modify] https://crrev.com/8893ec99d0180595cf950a2f7bad2869d9c394b7/content/browser/browser_context.cc
[modify] https://crrev.com/8893ec99d0180595cf950a2f7bad2869d9c394b7/content/browser/loader/resource_message_filter.cc
[modify] https://crrev.com/8893ec99d0180595cf950a2f7bad2869d9c394b7/content/browser/loader/shared_cors_origin_access_list_impl.cc
[modify] https://crrev.com/8893ec99d0180595cf950a2f7bad2869d9c394b7/content/public/browser/shared_cors_origin_access_list.h
[modify] https://crrev.com/8893ec99d0180595cf950a2f7bad2869d9c394b7/services/network/network_context.cc
[modify] https://crrev.com/8893ec99d0180595cf950a2f7bad2869d9c394b7/services/network/network_context.h
[modify] https://crrev.com/8893ec99d0180595cf950a2f7bad2869d9c394b7/services/network/public/mojom/network_context.mojom

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 13

Labels: merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/abeca37d816b48b549eb840a8231f704aac82c03

commit abeca37d816b48b549eb840a8231f704aac82c03
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Thu Dec 13 06:55:58 2018

OOR-CORS: Manage per-profile access list even for NetworkService

This patch makes BrowserContext manages per-profile CORS access lists
even if NetworkService is enabled, and use it to setup initial access
lists for the non-primary NetworkContext. It will also work for
restoring per-profile CORS settings on network service restarts.

This patch makes following tests work even with
--enable-features=OutOfBlinkCors,NetworkService.

- CrossOriginReadBlockingExtensionTest.ProgrammaticContentScriptVsAppCache
- CrossOriginReadBlockingExtensionTest.WebViewContentScript
- ExtensionWebRequestApiTest.ExtensionRequests
- PlatformAppBrowserTest.Isolation

Bug:  908324 , 891891,  895999 
Change-Id: Ib0cfc2f5633f25187366a4d7d63168d60ea51f71
Reviewed-on: https://chromium-review.googlesource.com/c/1351208
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612567}(cherry picked from commit 8893ec99d0180595cf950a2f7bad2869d9c394b7)

TBR=yhirano@chromium.org

Change-Id: Ib0cfc2f5633f25187366a4d7d63168d60ea51f71
Reviewed-on: https://chromium-review.googlesource.com/c/1373370
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#319}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/abeca37d816b48b549eb840a8231f704aac82c03/chrome/browser/net/profile_network_context_service.cc
[modify] https://crrev.com/abeca37d816b48b549eb840a8231f704aac82c03/content/browser/browser_context.cc
[modify] https://crrev.com/abeca37d816b48b549eb840a8231f704aac82c03/content/browser/loader/resource_message_filter.cc
[modify] https://crrev.com/abeca37d816b48b549eb840a8231f704aac82c03/content/browser/loader/shared_cors_origin_access_list_impl.cc
[modify] https://crrev.com/abeca37d816b48b549eb840a8231f704aac82c03/content/public/browser/shared_cors_origin_access_list.h
[modify] https://crrev.com/abeca37d816b48b549eb840a8231f704aac82c03/services/network/network_context.cc
[modify] https://crrev.com/abeca37d816b48b549eb840a8231f704aac82c03/services/network/network_context.h
[modify] https://crrev.com/abeca37d816b48b549eb840a8231f704aac82c03/services/network/public/mojom/network_context.mojom

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/abeca37d816b48b549eb840a8231f704aac82c03

Commit: abeca37d816b48b549eb840a8231f704aac82c03
Author: toyoshim@chromium.org
Commiter: toyoshim@chromium.org
Date: 2018-12-13 06:55:58 +0000 UTC

OOR-CORS: Manage per-profile access list even for NetworkService

This patch makes BrowserContext manages per-profile CORS access lists
even if NetworkService is enabled, and use it to setup initial access
lists for the non-primary NetworkContext. It will also work for
restoring per-profile CORS settings on network service restarts.

This patch makes following tests work even with
--enable-features=OutOfBlinkCors,NetworkService.

- CrossOriginReadBlockingExtensionTest.ProgrammaticContentScriptVsAppCache
- CrossOriginReadBlockingExtensionTest.WebViewContentScript
- ExtensionWebRequestApiTest.ExtensionRequests
- PlatformAppBrowserTest.Isolation

Bug:  908324 , 891891,  895999 
Change-Id: Ib0cfc2f5633f25187366a4d7d63168d60ea51f71
Reviewed-on: https://chromium-review.googlesource.com/c/1351208
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612567}(cherry picked from commit 8893ec99d0180595cf950a2f7bad2869d9c394b7)

TBR=yhirano@chromium.org

Change-Id: Ib0cfc2f5633f25187366a4d7d63168d60ea51f71
Reviewed-on: https://chromium-review.googlesource.com/c/1373370
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#319}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment