New issue
Advanced search Search tips

Issue 803321 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Use scoped_refptr for the url context getter in the component updater

Project Member Reported by sorin@chromium.org, Jan 18 2018

Issue description

This change's purpose includes:
1. Changing the url context getters raw pointers to scoped_refptr.
2. Avoiding storing the system url context getter and only return it when needed to do a url request.

Obviously, using smart pointers is preferred in this case, since the url context getter is a ref counted object and manipulating such object by a raw pointer is risky.

Second, the component updater is instantiated early on during the browser process startup. Since the system url context getter is early acquired during the construction of the component updater and its dependencies. This context getter is only used much later on when making url fetcher requests, therefore, requesting such context as needed is desirable.

 

Comment 1 by sorin@chromium.org, Jan 18 2018

This was an unrelated issue that prompted working on this change: https://bugs.chromium.org/p/chromium/issues/detail?id=479898#c33
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 19 2018

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

commit 556147da66b83b40046a083c2c4015ed891e46ad
Author: Sorin Jianu <sorin@chromium.org>
Date: Fri Jan 19 21:55:21 2018

Use scoped_refptr for the url context getter in the component updater.

BUG= 803321 

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Iaf24028c071606c1565127b92372de6bc3c62da1
Reviewed-on: https://chromium-review.googlesource.com/868753
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530624}
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/chrome/browser/component_updater/chrome_component_updater_configurator.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/chrome/browser/component_updater/chrome_component_updater_configurator.h
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/chrome/browser/component_updater/chrome_component_updater_configurator_unittest.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/chrome/browser/extensions/updater/chrome_update_client_config.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/chrome/browser/extensions/updater/chrome_update_client_config.h
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/chrome/browser/policy/policy_browsertest.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/component_updater/configurator_impl.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/component_updater/configurator_impl.h
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/component_updater/configurator_impl_unittest.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/update_client/configurator.h
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/update_client/crx_downloader.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/update_client/crx_downloader.h
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/update_client/test_configurator.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/update_client/test_configurator.h
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/update_client/update_client_unittest.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/update_client/url_fetcher_downloader.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/update_client/url_fetcher_downloader.h
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/update_client/utils.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/components/update_client/utils.h
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/ios/chrome/browser/application_context_impl.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/ios/chrome/browser/component_updater/ios_component_updater_configurator.cc
[modify] https://crrev.com/556147da66b83b40046a083c2c4015ed891e46ad/ios/chrome/browser/component_updater/ios_component_updater_configurator.h

Comment 3 by sorin@chromium.org, Jan 22 2018

Status: Fixed (was: Started)

Sign in to add a comment