Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Starred by 1 user
Status: Fixed
Owner:
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment
iOS should use /components/image_fetcher/image_data_fetcher
Project Member Reported by gambard@chromium.org, Jan 23 2017 Back to list
To be consistent with the rest of chrome, Chrome on iOS should use the /components/image_fetcher/image_data_fetcher instead of ios/web/public/image_fetcher/image_data_fetcher.
 
Project Member Comment 1 by bugdroid1@chromium.org, Feb 2 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c3d18557ba0b9d0e31598ed62e7129f97a582c7

commit 3c3d18557ba0b9d0e31598ed62e7129f97a582c7
Author: gambard <gambard@chromium.org>
Date: Thu Feb 02 12:58:27 2017

Create the IOSImageDataFetcherWrapper

Create a wrapper of ImageDataFetcher for iOS.
This class has a method to fetches and returns the image decoded, if it is a
WebP image, as iOS does not decode WebP.

BUG= 683918 

Review-Url: https://codereview.chromium.org/2663213002
Cr-Commit-Position: refs/heads/master@{#447752}

[modify] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/components/BUILD.gn
[add] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/components/image_fetcher/ios/BUILD.gn
[add] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/components/image_fetcher/ios/DEPS
[add] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/components/image_fetcher/ios/OWNERS
[add] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h
[add] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/components/image_fetcher/ios/ios_image_data_fetcher_wrapper.mm
[add] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/components/image_fetcher/ios/ios_image_data_fetcher_wrapper_unittest.mm
[modify] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/ios/chrome/browser/suggestions/BUILD.gn
[modify] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/ios/chrome/browser/suggestions/image_fetcher_impl.h
[modify] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/ios/chrome/browser/suggestions/image_fetcher_impl.mm
[modify] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/ios/web/public/image_fetcher/webp_decoder.h
[modify] https://crrev.com/3c3d18557ba0b9d0e31598ed62e7129f97a582c7/ios/web/public/image_fetcher/webp_decoder.mm

Project Member Comment 3 by bugdroid1@chromium.org, Feb 14 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/911c2949fcb7a8362449e8af1d60bbb93d3cfab5

commit 911c2949fcb7a8362449e8af1d60bbb93d3cfab5
Author: gambard <gambard@chromium.org>
Date: Tue Feb 14 10:43:40 2017

Use IOSImageDataFetcherWrapper for favicon

Use IOSImageDataFetcher instead of ImageDataFetcher for downloading the favicon
on iOS. This allows the download to be done in the favicon driver instead of the
WebState.
The favicon needs the http response code for optimization (if the favicon does
not exist we try to download it only once).

BUG= 683918 

Review-Url: https://codereview.chromium.org/2677993002
Cr-Commit-Position: refs/heads/master@{#450317}

[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/favicon/ios/BUILD.gn
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/favicon/ios/DEPS
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/favicon/ios/web_favicon_driver.h
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/favicon/ios/web_favicon_driver.mm
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/image_fetcher/image_data_fetcher.cc
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/image_fetcher/image_data_fetcher.h
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/image_fetcher/image_data_fetcher_unittest.cc
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/image_fetcher/request_metadata.cc
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/image_fetcher/request_metadata.h
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/components/image_fetcher/request_metadata_unittest.cc
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/ios/web/public/test/fakes/test_web_state.h
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/ios/web/public/test/fakes/test_web_state.mm
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/ios/web/public/web_state/web_state.h
[delete] https://crrev.com/afdfa8058e3d9e8fcfab6fd09ebf68d9ec31d3c7/ios/web/web_state/DEPS
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/911c2949fcb7a8362449e8af1d60bbb93d3cfab5/ios/web/web_state/web_state_impl.mm

Project Member Comment 4 by bugdroid1@chromium.org, Feb 14 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8412e37d0409d43e915a23b87f5cda464bae507a

commit 8412e37d0409d43e915a23b87f5cda464bae507a
Author: msramek <msramek@chromium.org>
Date: Tue Feb 14 11:43:38 2017

Revert of Use IOSImageDataFetcherWrapper for favicon (patchset #9 id:160001 of https://codereview.chromium.org/2677993002/ )

Reason for revert:
A compilation error shown on several iOS bots, e.g. here: https://build.chromium.org/p/chromium.mac/builders/ios-simulator-xcode-clang/builds/11111/steps/compile/logs/stdio

The following lines:
=====================================================================================================================
../../components/favicon/ios/web_favicon_driver.mm:134:7: error: no matching constructor for initialization of 'image_fetcher::IOSImageDataFetcherWrapper'
      image_fetcher_(web_state->GetBrowserState()->GetRequestContext(),
      ^              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h:37:3: note: candidate constructor not viable: cannot convert argument of incomplete type 'base::SequencedWorkerPool *' to 'const scoped_refptr<base::TaskRunner>'
  IOSImageDataFetcherWrapper(
  ^
../../components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h:70:28: note: candidate constructor not viable: requires 1 argument, but 2 were provided
  DISALLOW_COPY_AND_ASSIGN(IOSImageDataFetcherWrapper);
=====================================================================================================================

seem to point to this CL.

Although the buildbot failure email pointed to another CL (https://codereview.chromium.org/2690533002/), I think that one is a red herring, and I'll proceed to revert this one.

Original issue's description:
> Use IOSImageDataFetcherWrapper for favicon
>
> Use IOSImageDataFetcher instead of ImageDataFetcher for downloading the favicon
> on iOS. This allows the download to be done in the favicon driver instead of the
> WebState.
> The favicon needs the http response code for optimization (if the favicon does
> not exist we try to download it only once).
>
> BUG= 683918 
>
> Review-Url: https://codereview.chromium.org/2677993002
> Cr-Commit-Position: refs/heads/master@{#450317}
> Committed: https://chromium.googlesource.com/chromium/src/+/911c2949fcb7a8362449e8af1d60bbb93d3cfab5

TBR=treib@chromium.org,markusheintz@chromium.org,sdefresne@chromium.org,rohitrao@chromium.org,eugenebut@chromium.org,reed@google.com,bsalomon@google.com,gambard@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 683918 

Review-Url: https://codereview.chromium.org/2693043003
Cr-Commit-Position: refs/heads/master@{#450323}

[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/favicon/ios/BUILD.gn
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/favicon/ios/DEPS
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/favicon/ios/web_favicon_driver.h
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/favicon/ios/web_favicon_driver.mm
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/image_fetcher/image_data_fetcher.cc
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/image_fetcher/image_data_fetcher.h
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/image_fetcher/image_data_fetcher_unittest.cc
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/image_fetcher/request_metadata.cc
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/image_fetcher/request_metadata.h
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/components/image_fetcher/request_metadata_unittest.cc
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/ios/web/public/test/fakes/test_web_state.h
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/ios/web/public/test/fakes/test_web_state.mm
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/ios/web/public/web_state/web_state.h
[add] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/ios/web/web_state/DEPS
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/8412e37d0409d43e915a23b87f5cda464bae507a/ios/web/web_state/web_state_impl.mm

Project Member Comment 5 by bugdroid1@chromium.org, Feb 14 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea69b8739b1bf1a220ea541cca7886597c04bf0b

commit ea69b8739b1bf1a220ea541cca7886597c04bf0b
Author: gambard <gambard@chromium.org>
Date: Tue Feb 14 14:05:56 2017

Use IOSImageDataFetcherWrapper for favicon

Use IOSImageDataFetcher instead of ImageDataFetcher for downloading the favicon
on iOS. This allows the download to be done in the favicon driver instead of the
WebState.
The favicon needs the http response code for optimization (if the favicon does
not exist we try to download it only once).

BUG= 683918 

Review-Url: https://codereview.chromium.org/2677993002
Cr-Original-Commit-Position: refs/heads/master@{#450317}
Committed: https://chromium.googlesource.com/chromium/src/+/911c2949fcb7a8362449e8af1d60bbb93d3cfab5
Review-Url: https://codereview.chromium.org/2677993002
Cr-Commit-Position: refs/heads/master@{#450345}

[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/favicon/ios/BUILD.gn
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/favicon/ios/DEPS
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/favicon/ios/web_favicon_driver.h
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/favicon/ios/web_favicon_driver.mm
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/image_fetcher/image_data_fetcher.cc
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/image_fetcher/image_data_fetcher.h
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/image_fetcher/image_data_fetcher_unittest.cc
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/image_fetcher/request_metadata.cc
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/image_fetcher/request_metadata.h
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/components/image_fetcher/request_metadata_unittest.cc
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/ios/web/public/test/fakes/test_web_state.h
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/ios/web/public/test/fakes/test_web_state.mm
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/ios/web/public/web_state/web_state.h
[delete] https://crrev.com/bfb9503efad1c03a8ab1a26cf60f779609aa6cca/ios/web/web_state/DEPS
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/ea69b8739b1bf1a220ea541cca7886597c04bf0b/ios/web/web_state/web_state_impl.mm

Project Member Comment 6 by bugdroid1@chromium.org, Feb 16 2017
The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/a6f5d6fe7f31980f8e55154dbe41b93869b87fae

commit a6f5d6fe7f31980f8e55154dbe41b93869b87fae
Author: gambard <gambard@google.com>
Date: Thu Feb 16 17:10:19 2017

Project Member Comment 7 by bugdroid1@chromium.org, Feb 16 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b4fcbd5a15588d9823c4d85b7030fc2669882e6

commit 3b4fcbd5a15588d9823c4d85b7030fc2669882e6
Author: gambard <gambard@chromium.org>
Date: Thu Feb 16 18:34:27 2017

Use IOSImageDataFetcherWrapper

Uses IOSImageDataFetcherWrapper in components instead of ImageDataFetcher in
ios/web/public.

BUG= 683918 

Review-Url: https://codereview.chromium.org/2669173002
Cr-Commit-Position: refs/heads/master@{#451016}

[modify] https://crrev.com/3b4fcbd5a15588d9823c4d85b7030fc2669882e6/ios/chrome/browser/native_app_launcher/BUILD.gn
[modify] https://crrev.com/3b4fcbd5a15588d9823c4d85b7030fc2669882e6/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm
[modify] https://crrev.com/3b4fcbd5a15588d9823c4d85b7030fc2669882e6/ios/chrome/browser/ui/omnibox/BUILD.gn
[modify] https://crrev.com/3b4fcbd5a15588d9823c4d85b7030fc2669882e6/ios/chrome/browser/ui/omnibox/omnibox_popup_material_view_controller.h
[modify] https://crrev.com/3b4fcbd5a15588d9823c4d85b7030fc2669882e6/ios/chrome/browser/ui/omnibox/omnibox_popup_material_view_controller.mm
[modify] https://crrev.com/3b4fcbd5a15588d9823c4d85b7030fc2669882e6/ios/chrome/browser/ui/omnibox/omnibox_popup_view_ios.mm
[modify] https://crrev.com/3b4fcbd5a15588d9823c4d85b7030fc2669882e6/ios/chrome/browser/ui/settings/BUILD.gn
[modify] https://crrev.com/3b4fcbd5a15588d9823c4d85b7030fc2669882e6/ios/chrome/browser/ui/settings/native_apps_collection_view_controller.mm
[modify] https://crrev.com/3b4fcbd5a15588d9823c4d85b7030fc2669882e6/ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_metadata.mm
[modify] https://crrev.com/3b4fcbd5a15588d9823c4d85b7030fc2669882e6/ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h

Project Member Comment 8 by bugdroid1@chromium.org, Feb 17 2017
The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/9213f7cece60b038d986bc88261e0441386fc59a

commit 9213f7cece60b038d986bc88261e0441386fc59a
Author: gambard <gambard@google.com>
Date: Fri Feb 17 09:54:32 2017

Project Member Comment 10 by bugdroid1@chromium.org, Feb 20 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1

commit a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1
Author: gambard <gambard@chromium.org>
Date: Mon Feb 20 11:28:30 2017

Move WebpDecoder from ios/web to components/image_fetcher

As iOS now use the ImageFetcher from components, the WebPDecoder should also
live in components/image_fetcher/ios.

BUG= 683918 

Review-Url: https://codereview.chromium.org/2699633006
Cr-Commit-Position: refs/heads/master@{#451596}

[modify] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/image_fetcher/ios/BUILD.gn
[modify] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/image_fetcher/ios/DEPS
[modify] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/image_fetcher/ios/ios_image_data_fetcher_wrapper.mm
[modify] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/image_fetcher/ios/ios_image_data_fetcher_wrapper_unittest.mm
[rename] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/image_fetcher/ios/webp_decoder.h
[rename] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/image_fetcher/ios/webp_decoder.mm
[rename] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/image_fetcher/ios/webp_decoder_unittest.mm
[rename] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/test/data/webp_transcode/OWNERS
[rename] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/test/data/webp_transcode/test.jpg
[rename] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/test/data/webp_transcode/test.webp
[rename] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/test/data/webp_transcode/test_alpha.png
[rename] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/test/data/webp_transcode/test_alpha.webp
[rename] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/test/data/webp_transcode/test_small.tiff
[rename] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/components/test/data/webp_transcode/test_small.webp
[modify] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/ios/chrome/browser/suggestions/BUILD.gn
[modify] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/ios/chrome/browser/suggestions/ios_image_decoder_impl.mm
[modify] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/ios/chrome/browser/ui/omnibox/BUILD.gn
[modify] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/ios/chrome/test/BUILD.gn
[modify] https://crrev.com/a1313959f214f8eea4efa3fd9fe6ed0ee7cffaf1/ios/web/BUILD.gn
[delete] https://crrev.com/e859ab1300be6e103b62bc5409214e5584287815/ios/web/public/image_fetcher/BUILD.gn
[delete] https://crrev.com/e859ab1300be6e103b62bc5409214e5584287815/ios/web/public/image_fetcher/DEPS
[delete] https://crrev.com/e859ab1300be6e103b62bc5409214e5584287815/ios/web/public/image_fetcher/OWNERS

Status: Fixed
Sign in to add a comment