New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 624761 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Last visit 16 days ago
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 625081



Sign in to add a comment

Move all image_fetcher related code into the component directory

Project Member Reported by markusheintz@chromium.org, Jun 30 2016

Issue description

All image_fetcher and image_decoder related code should live in the component directory.

Currently e.g. the platform dependent image decoders implementations live in 
//chrome/browser/search/suggestions/    and
//ios/chrome/browser/suggestions/




 

Comment 1 by treib@chromium.org, Jun 30 2016

Actually I don't think so. The non-ios implementation uses chrome/browser/image_decoder.h, so it can't move into the component. Not sure about the ios one.

They definitely should move out of /suggestions though.
The iOS one can be moved easily because it does not have a dependency on chrome/browser/.

Bummer I forgot about this the image_decoder. :-(

I will still continue to investigate. 
Actually I guess we could put the image_decoder of chrome in components/image_fetcher or in it's own component.  

Comment 4 by treib@chromium.org, Jul 1 2016

Right, that should be possible! It'll be a bit of work though to pull the other dependencies on chrome/ out of the image decoder. Not sure it's worth the effort and churn (but up to you :) ).
I think dependencies on content/ from components/ also need some special care?
Cc: vitaliii@chromium.org
Blockedon: 625081
Cc: -vitaliii@chromium.org

Comment 8 by mastiz@chromium.org, Mar 21 2017

This got on my way due to a cyclic dependency and I started to move the iOS imagedecoder, might need to move the chrome equivalent as well.
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 21 2017

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

commit a31cd7215e84eee41286dc7f08020c740515ebe5
Author: mastiz <mastiz@chromium.org>
Date: Tue Mar 21 15:19:58 2017

Move iOS image_fetcher::ImageDecoder to layered component

In general, ImageDecoder is not specific to suggestions and hence the
code does not belong ios/chrome/browser/suggestions.

In particular, we do this to avoid a dependency cycle in a future CL,
due to the integration of ImageFetcher in FaviconService, causing:
ERROR Dependency cycle:
  //ios/chrome/browser/sync:sync ->
  //ios/chrome/browser/bookmarks:bookmarks ->
  //ios/chrome/browser/favicon:favicon ->
  //ios/chrome/browser/suggestions:suggestions ->
  //ios/chrome/browser/sync:sync

BUG= 644102 ,624761

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

[modify] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/components/image_fetcher/ios/BUILD.gn
[modify] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/components/image_fetcher/ios/DEPS
[add] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/components/image_fetcher/ios/ios_image_decoder_impl.h
[rename] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/components/image_fetcher/ios/ios_image_decoder_impl.mm
[rename] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/components/image_fetcher/ios/ios_image_decoder_impl_unittest.mm
[modify] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/ios/chrome/browser/ntp_snippets/BUILD.gn
[modify] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc
[modify] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/ios/chrome/browser/ntp_tiles/BUILD.gn
[modify] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/ios/chrome/browser/ntp_tiles/ios_most_visited_sites_factory.cc
[modify] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/ios/chrome/browser/suggestions/BUILD.gn
[delete] https://crrev.com/33d8074e449407ac84ad0e6f3ea7ab3dbda19f28/ios/chrome/browser/suggestions/ios_image_decoder_impl.h
[modify] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/ios/chrome/browser/suggestions/suggestions_service_factory.mm
[modify] https://crrev.com/a31cd7215e84eee41286dc7f08020c740515ebe5/ios/chrome/test/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 21 2017

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

commit f4d088af9db25dfb569645655da829c28f84ee52
Author: mastiz <mastiz@chromium.org>
Date: Tue Mar 21 17:41:40 2017

Move common ImageFetcher component files to core/

Let's follow the directory structure of layered components.

TBR=mathp@chromium.org,sdefresne@chromium.org,jam@chromium.org
BUG=624761

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

[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/android/logo_bridge.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/chromeos/hats/hats_notification_controller.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/chromeos/hats/hats_notification_controller_unittest.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/search/instant_service.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/search/suggestions/image_decoder_impl.h
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/search/suggestions/image_fetcher_impl_browsertest.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/search/suggestions/suggestions_service_factory.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/search/thumbnail_source.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/chrome/browser/ui/webui/ntp_tiles_internals_ui.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/BUILD.gn
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/BUILD.gn
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/image_data_fetcher.cc
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/image_data_fetcher.h
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/image_data_fetcher_unittest.cc
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/image_decoder.h
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/image_fetcher.h
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/image_fetcher_delegate.h
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/image_fetcher_impl.cc
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/image_fetcher_impl.h
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/request_metadata.cc
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/request_metadata.h
[rename] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/core/request_metadata_unittest.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/ios/BUILD.gn
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/image_fetcher/ios/ios_image_decoder_impl.h
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/ntp_snippets/BUILD.gn
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/ntp_tiles/BUILD.gn
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/ntp_tiles/icon_cacher_impl.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/ntp_tiles/icon_cacher_impl_unittest.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/suggestions/BUILD.gn
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/suggestions/image_manager.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/suggestions/image_manager.h
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/components/suggestions/image_manager_unittest.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/ios/chrome/browser/ntp_snippets/BUILD.gn
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/ios/chrome/browser/ntp_tiles/BUILD.gn
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/ios/chrome/browser/ntp_tiles/ios_most_visited_sites_factory.cc
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/ios/chrome/browser/suggestions/BUILD.gn
[modify] https://crrev.com/f4d088af9db25dfb569645655da829c28f84ee52/ios/chrome/browser/suggestions/suggestions_service_factory.mm

Sign in to add a comment