New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 780956 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 737282



Sign in to add a comment

NetworkService related getters should auto-reconnect after crash

Project Member Reported by chongz@chromium.org, Nov 2 2017

Issue description

Example Getters:
1. StoragePartition::GetNetworkContext()
2. URLLoaderFactoryGetter
3. ChildURLLoaderFactoryGetter
4. (Restricted)CookieManagerGetter

See master bug for more detail.
 

Comment 1 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 8 2017

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

commit c583e670b028bd0a3b24970912332d82f21702fe
Author: Chong Zhang <chongz@chromium.org>
Date: Wed Nov 08 16:34:08 2017

Re-create out-of-process NetworkService after connection error

This CL:
 1.Reconnects |content::GetNetworkService()::g_network_service|
   when |encountered_error()| is true (e.g. Crashed).
 2.Ands a browser test to ensure out-of-process NetworkService is
   accessible and functional after crash.

Note:
 * This CL only covers the basic availability of NetworkService. More
   tests will be added to ensure NetworkContext providers are working
   as expected (e.g. IOThread, StoragePartition).

Bug:  780956 
Change-Id: Ie1107d0c2dca5fbeaed7d6a90b7ac40103f1741d
Reviewed-on: https://chromium-review.googlesource.com/750223
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514853}
[add] https://crrev.com/c583e670b028bd0a3b24970912332d82f21702fe/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/c583e670b028bd0a3b24970912332d82f21702fe/content/public/browser/network_service_instance.cc
[modify] https://crrev.com/c583e670b028bd0a3b24970912332d82f21702fe/content/public/common/network_service_test.mojom
[modify] https://crrev.com/c583e670b028bd0a3b24970912332d82f21702fe/content/public/test/network_service_test_helper.cc
[modify] https://crrev.com/c583e670b028bd0a3b24970912332d82f21702fe/content/test/BUILD.gn

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 15 2017

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

commit 0219249755560000071aa9d2abfce8787e477cd7
Author: Chong Zhang <chongz@chromium.org>
Date: Wed Nov 15 18:56:48 2017

Handle NetworkContext error in StoragePartitionImpl and SystemNetworkContextManager

This CL:
 1.Adds reconnect logic to |StoragePartitionImpl::GetNetworkContext()|
   and |SystemNetworkContextManager::GetContext()| to makes sure they return
   valid interface after crash.
 2.Adds browser tests to ensure the new |NetworkContext| can make URL
   requests.

Note:
 * This CL only makes sure the returned |NetworkContext| is valid. More
   tests will be added to ensure other NetworkContext providers
   (e.g. IOThread) and |URLLoaderFactoryGetter| is working.

Bug:  780956 
Change-Id: Ie8bc4e2bb2e86c5e72f946eef1eb84fb8102910f
Reviewed-on: https://chromium-review.googlesource.com/758831
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516766}
[add] https://crrev.com/0219249755560000071aa9d2abfce8787e477cd7/chrome/browser/chrome_network_service_restart_browsertest.cc
[modify] https://crrev.com/0219249755560000071aa9d2abfce8787e477cd7/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/0219249755560000071aa9d2abfce8787e477cd7/chrome/browser/net/system_network_context_manager.h
[modify] https://crrev.com/0219249755560000071aa9d2abfce8787e477cd7/chrome/test/BUILD.gn
[modify] https://crrev.com/0219249755560000071aa9d2abfce8787e477cd7/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/0219249755560000071aa9d2abfce8787e477cd7/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/0219249755560000071aa9d2abfce8787e477cd7/content/browser/storage_partition_impl.h
[modify] https://crrev.com/0219249755560000071aa9d2abfce8787e477cd7/content/public/browser/storage_partition.h
[modify] https://crrev.com/0219249755560000071aa9d2abfce8787e477cd7/content/public/test/test_storage_partition.cc
[modify] https://crrev.com/0219249755560000071aa9d2abfce8787e477cd7/content/public/test/test_storage_partition.h

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 16 2017

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

commit 99c58c71df79db36341ad73c7fd880347b3b25a9
Author: Guido Urdaneta <guidou@chromium.org>
Date: Thu Nov 16 09:38:47 2017

Revert "Handle NetworkContext error in StoragePartitionImpl and SystemNetworkContextManager"

This reverts commit 0219249755560000071aa9d2abfce8787e477cd7.

Reason for revert: 
The following tests started failing consistently right after this CL landed:
ChromeNetworkServiceRestartBrowserTest.StoragePartitionGetNetworkContext
ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext

First failure: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/64657


Sample logs:
[330/331] ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext (TIMED OUT)
[331/331] PrerenderBrowserTestWithExtensions.StreamsTest (6672 ms)
Retrying 1 test (retry #2)
Still waiting for the following processes to finish:
	".\browser_tests.exe" --brave-new-test-launcher --cfi-diag=0 --gtest_also_run_disabled_tests --gtest_filter=ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext --single_process --test-launcher-bot-mode --test-launcher-output="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\results4976_25891\test_results.xml" --test-launcher-summary-output="e:\b\s\w\io7tx1xz\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\d4976_10509"
Still waiting for the following processes to finish:
	".\browser_tests.exe" --brave-new-test-launcher --cfi-diag=0 --gtest_also_run_disabled_tests --gtest_filter=ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext --single_process --test-launcher-bot-mode --test-launcher-output="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\results4976_25891\test_results.xml" --test-launcher-summary-output="e:\b\s\w\io7tx1xz\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\d4976_10509"
Still waiting for the following processes to finish:
	".\browser_tests.exe" --brave-new-test-launcher --cfi-diag=0 --gtest_also_run_disabled_tests --gtest_filter=ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext --single_process --test-launcher-bot-mode --test-launcher-output="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\results4976_25891\test_results.xml" --test-launcher-summary-output="e:\b\s\w\io7tx1xz\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\d4976_10509"
[ RUN      ] ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext
[1320:5532:1115/211725.385:WARNING:chrome_browser_main_win.cc(613)] Command line too long for RegisterApplicationRestart:  --brave-new-test-launcher --cfi-diag=0 --gtest_also_run_disabled_tests --gtest_filter=ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext --single_process --test-launcher-bot-mode --test-launcher-output="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\results4976_25891\test_results.xml" --test-launcher-summary-output="e:\b\s\w\io7tx1xz\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\d4976_10509" --disable-offline-auto-reload --no-first-run --no-default-browser-check --enable-logging=stderr --safebrowsing-disable-auto-update --disable-default-apps --wm-window-animations-disabled --disable-component-update --test-type=browser --force-color-profile=srgb --disable-zero-browsers-open-for-tests --ipc-connection-timeout=45 --allow-file-access-from-files --dom-automation --log-gpu-control-list-decisions --disable-backgrounding-occluded-windows --disable-gl-drawing-for-tests --override-use-software-gl-for-tests --force-color-profile=srgb --enable-features=NetworkService,TestFeatureForBrowserTest1 --disable-features=NetworkPrediction,TestFeatureForBrowserTest2 --flag-switches-begin --flag-switches-end --restore-last-session about:blank
[5536:4644:1115/211726.031:INFO:media_foundation_video_encode_accelerator_win.cc(370)] Windows versions earlier than 8 are not supported.
[332/332] ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext (TIMED OUT)
Retrying 1 test (retry #3)
Still waiting for the following processes to finish:
	".\browser_tests.exe" --brave-new-test-launcher --cfi-diag=0 --gtest_also_run_disabled_tests --gtest_filter=ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext --single_process --test-launcher-bot-mode --test-launcher-output="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\results4976_29411\test_results.xml" --test-launcher-summary-output="e:\b\s\w\io7tx1xz\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\d4976_16217"
Still waiting for the following processes to finish:
	".\browser_tests.exe" --brave-new-test-launcher --cfi-diag=0 --gtest_also_run_disabled_tests --gtest_filter=ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext --single_process --test-launcher-bot-mode --test-launcher-output="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\results4976_29411\test_results.xml" --test-launcher-summary-output="e:\b\s\w\io7tx1xz\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\d4976_16217"
Still waiting for the following processes to finish:
	".\browser_tests.exe" --brave-new-test-launcher --cfi-diag=0 --gtest_also_run_disabled_tests --gtest_filter=ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext --single_process --test-launcher-bot-mode --test-launcher-output="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\results4976_29411\test_results.xml" --test-launcher-summary-output="e:\b\s\w\io7tx1xz\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\d4976_16217"
[ RUN      ] ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext
[4852:4412:1115/211810.497:WARNING:chrome_browser_main_win.cc(613)] Command line too long for RegisterApplicationRestart:  --brave-new-test-launcher --cfi-diag=0 --gtest_also_run_disabled_tests --gtest_filter=ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext --single_process --test-launcher-bot-mode --test-launcher-output="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\results4976_29411\test_results.xml" --test-launcher-summary-output="e:\b\s\w\io7tx1xz\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir4976_15072\d4976_16217" --disable-offline-auto-reload --no-first-run --no-default-browser-check --enable-logging=stderr --safebrowsing-disable-auto-update --disable-default-apps --wm-window-animations-disabled --disable-component-update --test-type=browser --force-color-profile=srgb --disable-zero-browsers-open-for-tests --ipc-connection-timeout=45 --allow-file-access-from-files --dom-automation --log-gpu-control-list-decisions --disable-backgrounding-occluded-windows --disable-gl-drawing-for-tests --override-use-software-gl-for-tests --force-color-profile=srgb --enable-features=NetworkService,TestFeatureForBrowserTest1 --disable-features=NetworkPrediction,TestFeatureForBrowserTest2 --flag-switches-begin --flag-switches-end --restore-last-session about:blank
[2460:2636:1115/211811.151:INFO:media_foundation_video_encode_accelerator_win.cc(370)] Windows versions earlier than 8 are not supported.
[333/333] ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext (TIMED OUT)
1 test timed out:
    ChromeNetworkServiceRestartBrowserTest.SystemNetworkContextManagerGetContext (../../chrome/browser/chrome_network_service_restart_browsertest.cc:96)
+------------------------------------------------------------------------+

Original change's description:
> Handle NetworkContext error in StoragePartitionImpl and SystemNetworkContextManager
> 
> This CL:
>  1.Adds reconnect logic to |StoragePartitionImpl::GetNetworkContext()|
>    and |SystemNetworkContextManager::GetContext()| to makes sure they return
>    valid interface after crash.
>  2.Adds browser tests to ensure the new |NetworkContext| can make URL
>    requests.
> 
> Note:
>  * This CL only makes sure the returned |NetworkContext| is valid. More
>    tests will be added to ensure other NetworkContext providers
>    (e.g. IOThread) and |URLLoaderFactoryGetter| is working.
> 
> Bug:  780956 
> Change-Id: Ie8bc4e2bb2e86c5e72f946eef1eb84fb8102910f
> Reviewed-on: https://chromium-review.googlesource.com/758831
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Commit-Queue: Chong Zhang <chongz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#516766}

TBR=jam@chromium.org,yzshen@chromium.org,mmenke@chromium.org,chongz@chromium.org

Change-Id: If30e7394e6f2e4a9c14d7671a4106243aa72b9c1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  780956 
Reviewed-on: https://chromium-review.googlesource.com/774398
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517036}
[delete] https://crrev.com/ec0158516af927aeb0e16229e5b3768cfd40fd52/chrome/browser/chrome_network_service_restart_browsertest.cc
[modify] https://crrev.com/99c58c71df79db36341ad73c7fd880347b3b25a9/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/99c58c71df79db36341ad73c7fd880347b3b25a9/chrome/browser/net/system_network_context_manager.h
[modify] https://crrev.com/99c58c71df79db36341ad73c7fd880347b3b25a9/chrome/test/BUILD.gn
[modify] https://crrev.com/99c58c71df79db36341ad73c7fd880347b3b25a9/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/99c58c71df79db36341ad73c7fd880347b3b25a9/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/99c58c71df79db36341ad73c7fd880347b3b25a9/content/browser/storage_partition_impl.h
[modify] https://crrev.com/99c58c71df79db36341ad73c7fd880347b3b25a9/content/public/browser/storage_partition.h
[modify] https://crrev.com/99c58c71df79db36341ad73c7fd880347b3b25a9/content/public/test/test_storage_partition.cc
[modify] https://crrev.com/99c58c71df79db36341ad73c7fd880347b3b25a9/content/public/test/test_storage_partition.h

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 7 2017

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

commit 5f46827021c628c8149297292f07a0e13ef9d959
Author: Chong Zhang <chongz@chromium.org>
Date: Thu Dec 07 23:59:14 2017

Reland "Handle NetworkContext error in StoragePartitionImpl and SystemNetworkContextManager"

Based on the original CL, the reland:
1. Use |Process::Terminate()| instead of |CHECK()| to avoid 'Fatal error' dialog on
   Windows debug.
2. Use '/echoheader' instead of '/echo' to avoid a net/disk_cache issue
   (https://crbug.com/792255) on Windows release.

Note:
*  '/echoheader' uses 'no-cache' according to
   'default_handlers.cc::RegisterDefaultHandlers()'.

Original change's description:
> Handle NetworkContext error in StoragePartitionImpl and SystemNetworkContextManager
>
> This CL:
>  1.Adds reconnect logic to |StoragePartitionImpl::GetNetworkContext()|
>    and |SystemNetworkContextManager::GetContext()| to makes sure they return
>    valid interface after crash.
>  2.Adds browser tests to ensure the new |NetworkContext| can make URL
>    requests.
>
> Note:
>  * This CL only makes sure the returned |NetworkContext| is valid. More
>    tests will be added to ensure other NetworkContext providers
>    (e.g. IOThread) and |URLLoaderFactoryGetter| is working.
>
> Bug:  780956 
> Change-Id: Ie8bc4e2bb2e86c5e72f946eef1eb84fb8102910f
> Reviewed-on: https://chromium-review.googlesource.com/758831
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Commit-Queue: Chong Zhang <chongz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#516766}

Reverted on:
> Change-Id: If30e7394e6f2e4a9c14d7671a4106243aa72b9c1

Bug:  780956 
Change-Id: I5c86b02023e0540bc5bb6566c00d3bc9cb91fdbf
Reviewed-on: https://chromium-review.googlesource.com/801218
Commit-Queue: Chong Zhang <chongz@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522624}
[add] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/chrome/browser/chrome_network_service_restart_browsertest.cc
[modify] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/chrome/browser/net/system_network_context_manager.h
[modify] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/chrome/test/BUILD.gn
[modify] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/content/browser/storage_partition_impl.h
[modify] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/content/public/browser/storage_partition.h
[modify] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/content/public/test/network_service_test_helper.cc
[modify] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/content/public/test/test_storage_partition.cc
[modify] https://crrev.com/5f46827021c628c8149297292f07a0e13ef9d959/content/public/test/test_storage_partition.h

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 10 2017

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

commit c40a6ce516cb542ed97bb7a8c81bd0626bc9ed7e
Author: Chong Zhang <chongz@chromium.org>
Date: Sun Dec 10 03:00:28 2017

Handle URLLoaderFactory error in StoragePartitionImpl and SystemNetworkContextManager

This CL:
 1.Adds reconnect logic to |StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcess()|
   and |SystemNetworkContextManager::GetURLLoaderFactory()| to makes sure they return
   valid interface after crash.
 2.Moved two common test APIs into "browser_test_utils.h".
 3.Updated browser tests to run against restarted NetworkContext/URLLoaderFactory.

Note:
 * More tests will be added for other NetworkContext providers (e.g. IOThread),
   |URLLoaderFactoryGetter|, as well as Renderer related stuff.

Bug:  780956 
Change-Id: I78cd400263e79f2ba2b6088e108a499ee87ae1c3
Reviewed-on: https://chromium-review.googlesource.com/769855
Commit-Queue: Chong Zhang <chongz@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523017}
[modify] https://crrev.com/c40a6ce516cb542ed97bb7a8c81bd0626bc9ed7e/chrome/browser/chrome_network_service_restart_browsertest.cc
[modify] https://crrev.com/c40a6ce516cb542ed97bb7a8c81bd0626bc9ed7e/chrome/browser/net/network_context_configuration_browsertest.cc
[modify] https://crrev.com/c40a6ce516cb542ed97bb7a8c81bd0626bc9ed7e/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/c40a6ce516cb542ed97bb7a8c81bd0626bc9ed7e/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/c40a6ce516cb542ed97bb7a8c81bd0626bc9ed7e/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/c40a6ce516cb542ed97bb7a8c81bd0626bc9ed7e/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/c40a6ce516cb542ed97bb7a8c81bd0626bc9ed7e/content/public/test/browser_test_utils.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 4 2018

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

commit d4c9236441ff29b72f85754b9b0477f99e49c6fe
Author: Chong Zhang <chongz@chromium.org>
Date: Thu Jan 04 02:18:39 2018

Add reconnect logic to URLLoaderFactoryGetter

This CL:
1. Stores |StoragePartitionImpl*| in |URLLoaderFactoryGetter|, and
   re-creates |network_factory_| on connection error.
2. Adds basic 'content_browsertests' against |URLLoaderFactoryGetter|
   and |NavigationURLLoader|.

Note:
Other |URLLoaderFactoryGetter| consumers such as Service Worker and APP
Cache should be able to survive the crash as well. Will add more tests
in follow up CLs.

Bug:  780956 
Change-Id: I65678ece5c852b9454d9c800ff80e4d1cc2de178
Reviewed-on: https://chromium-review.googlesource.com/832947
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526900}
[modify] https://crrev.com/d4c9236441ff29b72f85754b9b0477f99e49c6fe/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/d4c9236441ff29b72f85754b9b0477f99e49c6fe/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/d4c9236441ff29b72f85754b9b0477f99e49c6fe/content/browser/url_loader_factory_getter.cc
[modify] https://crrev.com/d4c9236441ff29b72f85754b9b0477f99e49c6fe/content/browser/url_loader_factory_getter.h
[modify] https://crrev.com/d4c9236441ff29b72f85754b9b0477f99e49c6fe/content/public/test/simple_url_loader_test_helper.cc
[modify] https://crrev.com/d4c9236441ff29b72f85754b9b0477f99e49c6fe/content/public/test/simple_url_loader_test_helper.h

As a followup to the CL description above:
Service Worker doesn't seems to work properly with Network Service enabled, see  issue 715640 .
Will add browser tests for Service Worker after the issue was resolved. (Together with APP Cache even though it probably doesn't depend on that bug).
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 24 2018

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

commit 7306b0bb25d8e5b87eb5043bcfb82d304e1520d2
Author: Chong Zhang <chongz@chromium.org>
Date: Wed Jan 24 05:59:24 2018

Add reconnect logic to URLLoaderFactoryBundle

This CL:
1. Allows |RenderFrameHostImpl| to push new |URLLoaderFactoryBundle|
   to |RenderFrameImpl| after navigation so they can send updated
   Network Service factories in case of crash.
2. Adds a 'content_browsertests' to verify XHR still works after
   Network Service crash.

Bug:  780956 
Change-Id: I4323648c7475ff311b1e3825aeb804265ad6899c
Reviewed-on: https://chromium-review.googlesource.com/858428
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531449}
[modify] https://crrev.com/7306b0bb25d8e5b87eb5043bcfb82d304e1520d2/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/7306b0bb25d8e5b87eb5043bcfb82d304e1520d2/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/7306b0bb25d8e5b87eb5043bcfb82d304e1520d2/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/7306b0bb25d8e5b87eb5043bcfb82d304e1520d2/content/common/frame.mojom
[modify] https://crrev.com/7306b0bb25d8e5b87eb5043bcfb82d304e1520d2/content/common/url_loader_factory_bundle.cc
[modify] https://crrev.com/7306b0bb25d8e5b87eb5043bcfb82d304e1520d2/content/common/url_loader_factory_bundle.h
[modify] https://crrev.com/7306b0bb25d8e5b87eb5043bcfb82d304e1520d2/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/7306b0bb25d8e5b87eb5043bcfb82d304e1520d2/content/renderer/render_frame_impl.h
[modify] https://crrev.com/7306b0bb25d8e5b87eb5043bcfb82d304e1520d2/content/test/test_render_frame_host.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 30 2018

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

commit 5d9b34180a7eeeccb092ac3969e7355e2e769405
Author: Chong Zhang <chongz@chromium.org>
Date: Tue Jan 30 16:20:57 2018

Use same origin requests in NetworkServiceRestartBrowserTest.BasicXHR

This CL removes unnecessary cross-origin requests from the test.

Bug:  780956 
Change-Id: I7835d6363d8e1b5862889c5fd9992fdb1e064dac
Reviewed-on: https://chromium-review.googlesource.com/891550
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532901}
[modify] https://crrev.com/5d9b34180a7eeeccb092ac3969e7355e2e769405/content/browser/network_service_restart_browsertest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 28 2018

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

commit 4513fab1cd84c34f8f42e72277503ab6e8b8e9b7
Author: Chong Zhang <chongz@chromium.org>
Date: Wed Feb 28 18:50:18 2018

Introduce Tracked/HostChildURLLoaderFactoryBundle for Workers and frames with opener

This CL:
1. Introduce |Tracked/HostChildURLLoaderFactoryBundle| to track
   and post updates to cloned bundles in the event of Network
   Service crash.
2. Add browser tests to make sure WebWorkers and frames from
   'window.open()' can load URL after crash.

Related Discussions:
https://groups.google.com/a/chromium.org/forum/#!topic/network-service-dev/iWk6Dt9_GA0

Bug:  780956 
Change-Id: I3d0b585757e957949d9c4816799ffaa19045ac97
Reviewed-on: https://chromium-review.googlesource.com/912276
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539905}
[modify] https://crrev.com/4513fab1cd84c34f8f42e72277503ab6e8b8e9b7/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/4513fab1cd84c34f8f42e72277503ab6e8b8e9b7/content/renderer/BUILD.gn
[modify] https://crrev.com/4513fab1cd84c34f8f42e72277503ab6e8b8e9b7/content/renderer/loader/child_url_loader_factory_bundle.cc
[modify] https://crrev.com/4513fab1cd84c34f8f42e72277503ab6e8b8e9b7/content/renderer/loader/child_url_loader_factory_bundle.h
[add] https://crrev.com/4513fab1cd84c34f8f42e72277503ab6e8b8e9b7/content/renderer/loader/tracked_child_url_loader_factory_bundle.cc
[add] https://crrev.com/4513fab1cd84c34f8f42e72277503ab6e8b8e9b7/content/renderer/loader/tracked_child_url_loader_factory_bundle.h
[modify] https://crrev.com/4513fab1cd84c34f8f42e72277503ab6e8b8e9b7/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/4513fab1cd84c34f8f42e72277503ab6e8b8e9b7/content/renderer/render_frame_impl.h

Labels: Proj-Servicification
Status: Fixed (was: Started)
A summary of SharedURLLoaderFactory related interfaces that supports auto-reconnect:
1. Browser UI: |StoragePartition::GetURLLoaderFactoryForBrowserProcess()|.
2. Browser IO: |StoragePartition::GetURLLoaderFactoryForBrowserProcessIOThread()|.
3. Renderer: |RenderFrameImpl::loader_factories_| which can be cloned and tracked with auto-update.

CookieManager related:
1. Browser UI: |StoragePartition::GetCookieManagerForBrowserProcess()|.
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 10

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

commit 8a2e696672ae1a5045c4a78fb6c6656566a635a2
Author: Chong Zhang <chongz@chromium.org>
Date: Wed Oct 10 22:36:49 2018

Remove TODOs in TrackedChildURLLoaderFactoryBundle

This patch removes 2 TODOs as they are not longer applicable:

1. Currently |TrackedChildURLLoaderFactoryBundle::UpdateThisAndAllClones()|
takes an |URLLoaderFactoryBundleInfo| which doesn't have
|direct_network_factory_|, thus it cannot update its observers.
(Note: Only |ChildURLLoaderFactoryBundle| has |direct_network_factory_|)

We no longer consider this as an issue since |direct_network_factory_| is
only used by |RendererBlinkPlatformImpl| and doesn't rely on this codepath.

2. Removing the TODO about |SequencedTaskRunnerHandle::IsSet()| since there
doesn't seem to a plan of making sure all call sites have a
|SequencedTaskRunnerHandle|.

Bug:  780956 
Change-Id: I686d125aa6b94e40947be717eb5df095237b1dbd
Reviewed-on: https://chromium-review.googlesource.com/c/1272576
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598547}
[modify] https://crrev.com/8a2e696672ae1a5045c4a78fb6c6656566a635a2/content/renderer/loader/tracked_child_url_loader_factory_bundle.cc
[modify] https://crrev.com/8a2e696672ae1a5045c4a78fb6c6656566a635a2/content/renderer/loader/tracked_child_url_loader_factory_bundle.h

Sign in to add a comment