BackgroundFetchContext has redundant URLRequestContextGetter |
||
Issue descriptionThere is a scoped_refptr<URLRequestContextGetter> in BackgroundFetchContext that is never used after assignment but is there to prevent a crash: components_unittests --gtest_filter=ContentLoFiUIServiceTest.* This crash occurs because the test uses TestBrowserContext and this: SetThreadBundleOptions(content::TestBrowserThreadBundle::REAL_IO_THREAD); This seems to cause the URLRequestContextGetter to get destroyed on the IO thread where previous it was destroyed on the UI thread because BackgroundFetchDelegateProxy::Core gets destroyed on the UI thread via a posted task, which thus happens after the BrowserContext is destroyed. The problem with TestBrowserContext appears to be that it uses TestURLRequestContextGetter that uses a NullTaskRunner which claims to be able to run on any thread, which means the URLRequestContextGetter actually gets destroyed on any thread. https://cs.chromium.org/chromium/src/content/public/test/test_browser_context.cc?type=cs&sq=package:chromium&l=24
,
Sep 7 2017
I've bisected with a local change to StoragePartitionImplMap to not initialize the BackgroundFetchContext on the IO thread. That points to crrev/c//602707 as the problem. Before this change, the URLRequestContextGetter was not actually constructed at all and afterwards it is. This change checks to see if the site URL would use the default StoragePartition which it does by getting (and constructing if necessary) both the default storage partition and the site's storage partition and checking if they're the same. This seems like a fairly heavyweight way of checking and has some problems...
,
Sep 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c0d84930d30d860471477fb4bbe3f863535b0c8 commit 6c0d84930d30d860471477fb4bbe3f863535b0c8 Author: Dan Elphick <delphick@chromium.org> Date: Tue Sep 12 09:35:38 2017 Avoid creating StoragePartitions unnecessarily Instead check several conditions earlier to avoid getting (and thus creating) StoragePartitions that are never used. Also adds a new parameter to BrowserContext::GetStoragePartition to allow checking existence without creating the StoragePartition. Finally remove the hack in BackgroundFetchContext which preserved a scoped_refptr to URLRequestContextGetter to ensure it was deleted on the UI thread, now that the StoragePartition is not created and so won't create it. Bug: 762960 Change-Id: I57055c19f485be706485477125d1434e5ae99181 Reviewed-on: https://chromium-review.googlesource.com/654874 Commit-Queue: Dan Elphick <delphick@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#501226} [modify] https://crrev.com/6c0d84930d30d860471477fb4bbe3f863535b0c8/content/browser/background_fetch/background_fetch_context.cc [modify] https://crrev.com/6c0d84930d30d860471477fb4bbe3f863535b0c8/content/browser/background_fetch/background_fetch_context.h [modify] https://crrev.com/6c0d84930d30d860471477fb4bbe3f863535b0c8/content/browser/browser_context.cc [modify] https://crrev.com/6c0d84930d30d860471477fb4bbe3f863535b0c8/content/browser/renderer_host/render_process_host_impl.cc [modify] https://crrev.com/6c0d84930d30d860471477fb4bbe3f863535b0c8/content/browser/storage_partition_impl_map.cc [modify] https://crrev.com/6c0d84930d30d860471477fb4bbe3f863535b0c8/content/browser/storage_partition_impl_map.h [modify] https://crrev.com/6c0d84930d30d860471477fb4bbe3f863535b0c8/content/public/browser/browser_context.h
,
Sep 12 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by delph...@chromium.org
, Sep 7 2017