Migrate components/update_client/url_fetcher_downloader.cc |
|||||||
Issue description
,
Jun 20 2018
Related to issue 844973 (which is WIP).
,
Jul 12
Issue 862988 has been merged into this issue.
,
Jul 23
,
Jul 23
Antonio: can we prioritize this as a bunch of duplicate bugs have been filed on it? Thanks
,
Aug 1
on it, now, jam.
,
Aug 3
Issue 853810 has been merged into this issue.
,
Aug 6
,
Aug 6
,
Aug 9
FTR, this is CL https://crrev.com/c/1160725.
,
Aug 9
,
Aug 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8d36e8296a5882ac323aa2e4ffb3704d438b3838 commit 8d36e8296a5882ac323aa2e4ffb3704d438b3838 Author: Antonio Gomes <tonikitoo@igalia.com> Date: Thu Aug 09 23:14:59 2018 s13n: Expose an API to get the total of decompressed bytes of a response body While migrating some classes from URLFetcher to SimpleURLLoader, it is useful to have the size of the resulting decompressed body. In case of DownloadToString* calls, this is straightfoward: the length to the string is the size. However, in the case of DownloadTo{Temp}File calls, where only the resulting file path is passed to the completion callback, having an API to get the decompressed body size is useful. It also avoids sync calls to access the file data. This CL adds a SimpleURLLoader::GetContentSize API that provides such value. When used with SimpleURLLloader::SetAllowPartialResults(true), the API returns the total decompressed body size even in the case of errors. BUG=773295, 844972 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: Ia8cdb3326b5d5c74276e384f966a7e98f61a8c46 Reviewed-on: https://chromium-review.googlesource.com/1167683 Reviewed-by: Matt Menke <mmenke@chromium.org> Commit-Queue: Matt Menke <mmenke@chromium.org> Cr-Commit-Position: refs/heads/master@{#581952} [modify] https://crrev.com/8d36e8296a5882ac323aa2e4ffb3704d438b3838/services/network/public/cpp/simple_url_loader.cc [modify] https://crrev.com/8d36e8296a5882ac323aa2e4ffb3704d438b3838/services/network/public/cpp/simple_url_loader.h [modify] https://crrev.com/8d36e8296a5882ac323aa2e4ffb3704d438b3838/services/network/public/cpp/simple_url_loader_unittest.cc
,
Aug 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0807dd6facfb91816655d58e5c6f429fd9bb5590 commit 0807dd6facfb91816655d58e5c6f429fd9bb5590 Author: Antonio Gomes <tonikitoo@igalia.com> Date: Fri Aug 10 14:28:47 2018 Migrate components/update_client/url_fetcher_downloader.cc URLFetcher will stop working with advent of Network Service, and SimpleURLLoader is the replacement API for most clients. This CL migrates UrlFetcherDownloader away from URLFetcher. Note that UrlFetcherDownloader makes use of URLFetcher's "progress update" hook, in order to track fetch progresses. It turns out the progress values were not being used in production at all: UrlFetcherDownloader::OnURLFetchDownloadProgress used to instantiate a CrxDownloader::Result object, and pass it all the way to Component::StateDownloadingDiff::DownloadComplete and Component::StateDownloading::DownloadComplete. There, |result| parameter was being ignored in both methods. Given SimpleURLLoader's lack of ability to hook to a progress update callback at the time of doing this migration, and that the progress values were not being used in production code anyways, this CL replaces URLFetcher's progress upload hook by SimpleURLLoader's "response started" hook, to indicate that the load has been triggered, and update Component's status accordingly. Hence, references to |CrxDownloader::Result::downloaded_bytes| and |CrxDownloader::Result::total_bytes| were removed from CrxDownloaderTest.* (components/update_client/crx_downloader_unittest.cc). On the other hand, references to CrxDownloader::DownloadMetrics::downloaded_bytes and |CrxDownloader::DownloadMetrics::total_bytes| remain unchanged (update_client::BuildDownloadCompleteEventElement uses them). Bug:844972, 871211 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: I50410b46576263aec9a9a4b648a8a627f4832702 Reviewed-on: https://chromium-review.googlesource.com/1160725 Reviewed-by: Matt Menke <mmenke@chromium.org> Reviewed-by: Joshua Pawlicki <waffles@chromium.org> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Cr-Commit-Position: refs/heads/master@{#582150} [modify] https://crrev.com/0807dd6facfb91816655d58e5c6f429fd9bb5590/components/update_client/BUILD.gn [modify] https://crrev.com/0807dd6facfb91816655d58e5c6f429fd9bb5590/components/update_client/component.cc [modify] https://crrev.com/0807dd6facfb91816655d58e5c6f429fd9bb5590/components/update_client/crx_downloader.cc [modify] https://crrev.com/0807dd6facfb91816655d58e5c6f429fd9bb5590/components/update_client/crx_downloader.h [modify] https://crrev.com/0807dd6facfb91816655d58e5c6f429fd9bb5590/components/update_client/crx_downloader_unittest.cc [modify] https://crrev.com/0807dd6facfb91816655d58e5c6f429fd9bb5590/components/update_client/update_client_unittest.cc [modify] https://crrev.com/0807dd6facfb91816655d58e5c6f429fd9bb5590/components/update_client/url_fetcher_downloader.cc [modify] https://crrev.com/0807dd6facfb91816655d58e5c6f429fd9bb5590/components/update_client/url_fetcher_downloader.h
,
Aug 10
,
Aug 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/31237fb5f3bba96c2e260464de91e2fb6d9f29ac commit 31237fb5f3bba96c2e260464de91e2fb6d9f29ac Author: Antonio Gomes <tonikitoo@igalia.com> Date: Mon Aug 27 19:11:03 2018 Clean up CrxDownloader::Result and CrxDownloader::ProgressCallback This is a follow up of CL https://crrev.com/c/1136518 where, CrDownloader/UrlFetcherDownloader migrated from URLFetcher to SimpleURLLoader. As part of the migration, it was verified that CrxDownloader::Result had struct members only used in unit tests: |downloaded_bytes| and |total_bytes|. Additionally, the CrxDownloader::ProgressCallback did not use its |result| parameter. This CL removes the struct members from CrxDownloader::Result as well as their use (mocked up anyways) in unittests, and removes the unused |result| parameter from CrxDownloader::ProgressCallback. BUG= 844972 , 871211 Change-Id: I5b36c63784ff2632f71c1e5a0cd44b6e0c7c7f37 Reviewed-on: https://chromium-review.googlesource.com/1190324 Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Reviewed-by: Joshua Pawlicki <waffles@chromium.org> Cr-Commit-Position: refs/heads/master@{#586361} [modify] https://crrev.com/31237fb5f3bba96c2e260464de91e2fb6d9f29ac/components/update_client/background_downloader_win.cc [modify] https://crrev.com/31237fb5f3bba96c2e260464de91e2fb6d9f29ac/components/update_client/component.cc [modify] https://crrev.com/31237fb5f3bba96c2e260464de91e2fb6d9f29ac/components/update_client/component.h [modify] https://crrev.com/31237fb5f3bba96c2e260464de91e2fb6d9f29ac/components/update_client/crx_downloader.cc [modify] https://crrev.com/31237fb5f3bba96c2e260464de91e2fb6d9f29ac/components/update_client/crx_downloader.h [modify] https://crrev.com/31237fb5f3bba96c2e260464de91e2fb6d9f29ac/components/update_client/crx_downloader_unittest.cc [modify] https://crrev.com/31237fb5f3bba96c2e260464de91e2fb6d9f29ac/components/update_client/update_client_unittest.cc [modify] https://crrev.com/31237fb5f3bba96c2e260464de91e2fb6d9f29ac/components/update_client/url_fetcher_downloader.cc |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by dxie@google.com
, May 20 2018Status: Available (was: Untriaged)