New issue
Advanced search Search tips

Issue 740992 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

eliminate task runner injection in the component updater

Project Member Reported by sorin@chromium.org, Jul 11 2017

Issue description

The current component updater is injecting a task runner associated with
the blocking pool to avoid a dependency on content::BrowserThread::content::BrowserThread::GetBlockingPool.

The new task scheduler API makes it possible to program with thread pools and not depend on content. Therefore, the injection of task runner is not necessary 
anymore.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 11 2017

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

commit 72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6
Author: Sorin Jianu <sorin@chromium.org>
Date: Tue Jul 11 18:42:16 2017

Replace injected task runner in the component updater CRX downloader. 
Bug:  740992 

Change-Id: If31b639f9eba5968d7ddd0a688fc535cbf8ed527
Reviewed-on: https://chromium-review.googlesource.com/566160
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485684}
[modify] https://crrev.com/72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6/chrome/browser/component_updater/chrome_component_updater_configurator.cc
[modify] https://crrev.com/72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6/components/update_client/background_downloader_win.cc
[modify] https://crrev.com/72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6/components/update_client/background_downloader_win.h
[modify] https://crrev.com/72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6/components/update_client/component.cc
[modify] https://crrev.com/72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6/components/update_client/crx_downloader.cc
[modify] https://crrev.com/72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6/components/update_client/crx_downloader.h
[modify] https://crrev.com/72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6/components/update_client/crx_downloader_unittest.cc
[modify] https://crrev.com/72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6/components/update_client/update_client_unittest.cc
[modify] https://crrev.com/72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6/components/update_client/url_fetcher_downloader.cc
[modify] https://crrev.com/72d8fe0dd3fcaa259c3d034578dc7b6b4ef9bce6/components/update_client/url_fetcher_downloader.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 23 2017

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

commit ebd65246aab3b4b1cb5bd462388344195420078d
Author: Sorin Jianu <sorin@chromium.org>
Date: Sun Jul 23 02:00:58 2017

Eliminate more task runner injection in the component updater.

Bug:  740992 
Change-Id: Ib84f64eeef096850a48c8da632bd88fc74bffaf3
Reviewed-on: https://chromium-review.googlesource.com/581828
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488876}
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/chrome/browser/component_updater/component_patcher_operation_out_of_process.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/chrome/browser/component_updater/component_patcher_operation_out_of_process.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/chrome/browser/component_updater/component_patcher_operation_out_of_process_browsertest.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/chrome/browser/component_updater/cros_component_installer_unittest.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/chrome/browser/component_updater/subresource_filter_component_installer_unittest.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/chrome/browser/component_updater/supervised_user_whitelist_installer.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/chrome/browser/component_updater/supervised_user_whitelist_installer_unittest.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/component_updater/component_updater_service.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/component_updater/component_updater_service.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/component_updater/component_updater_service_internal.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/component_updater/component_updater_service_unittest.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/component_updater/default_component_installer.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/component_updater/default_component_installer.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/component_updater/default_component_installer_unittest.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/BUILD.gn
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/action_runner.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/action_runner.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/action_runner_win.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/background_downloader_win.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/background_downloader_win.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/component.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/component_patcher.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/component_patcher.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/component_patcher_operation.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/component_patcher_operation.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/component_patcher_unittest.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/component_patcher_unittest.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/component_unpacker.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/component_unpacker.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/component_unpacker_unittest.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/crx_downloader.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/crx_downloader.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/out_of_process_patcher.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/ping_manager.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/request_sender.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/task_send_uninstall_ping.cc
[add] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/task_traits.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/task_update.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/update_checker.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/update_checker.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/update_client.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/update_client_unittest.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/update_engine.cc
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/update_engine.h
[modify] https://crrev.com/ebd65246aab3b4b1cb5bd462388344195420078d/components/update_client/url_fetcher_downloader.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 26 2017

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

commit cc048f893dac3da0376abf7adc781b0452691a0d
Author: Sorin Jianu <sorin@chromium.org>
Date: Wed Jul 26 02:05:54 2017

Eliminate task runner injection from the configurator.

The new task scheduler API makes it possible to simplify the 
configurator and avoid injecting task runners.

Bug:  740992 
Change-Id: Iaabdc498993745372cee47193f16f1df4d09f7a2
Reviewed-on: https://chromium-review.googlesource.com/582030
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489528}
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/chrome/browser/component_updater/chrome_component_updater_configurator.cc
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/chrome/browser/extensions/updater/chrome_update_client_config.h
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/components/component_updater/component_updater_service_unittest.cc
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/components/component_updater/default_component_installer_unittest.cc
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/components/update_client/component_unpacker_unittest.cc
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/components/update_client/configurator.h
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/components/update_client/ping_manager_unittest.cc
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/components/update_client/request_sender_unittest.cc
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/components/update_client/test_configurator.cc
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/components/update_client/test_configurator.h
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/components/update_client/update_checker_unittest.cc
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/components/update_client/update_client_unittest.cc
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/extensions/browser/extensions_browser_client.cc
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/extensions/browser/updater/BUILD.gn
[delete] https://crrev.com/86aeb70b65bcd24058ab6f9f8038a8d8ea91de34/extensions/browser/updater/update_client_config.cc
[delete] https://crrev.com/86aeb70b65bcd24058ab6f9f8038a8d8ea91de34/extensions/browser/updater/update_client_config.h
[modify] https://crrev.com/cc048f893dac3da0376abf7adc781b0452691a0d/ios/chrome/browser/component_updater/ios_component_updater_configurator.cc

Comment 4 by sorin@chromium.org, Jul 27 2017

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 28 2017

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

commit 56f322b6332012f452e958cbefff9348dd853040
Author: Sorin Jianu <sorin@chromium.org>
Date: Tue Nov 28 19:20:42 2017

Fix assert due to threading issues in update_client::ActionRunner.

The change to use task scheduler in update_client has incorrectly made
some of the update_client::ActionRunner code run on the UI thread,
therefore, the unpacker code is asserting that IO is not allowed.

See change https://chromium-review.googlesource.com/c/chromium/src/+/770050

Bug:  740992 
Change-Id: I27a7ce650a1bf06ba76aac913d741c9adf5f6424
Reviewed-on: https://chromium-review.googlesource.com/792333
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519786}
[modify] https://crrev.com/56f322b6332012f452e958cbefff9348dd853040/components/update_client/action_runner.cc

Sign in to add a comment