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

Issue 735905 link

Starred by 1 user
Status: Fixed
Owner:
treib@chromium.org
Closed: Jul 2017
Cc:
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Cleanup: Remove dead thumbnail code

Project Member Reported by treib@chromium.org, Jun 22 2017 
"Thumbnail retargeting" was an attempt to take prettier thumbnails, by analyzing the actual image content and selecting the "most interesting" parts (see bug 245455 and  bug 155269 ). However, the code was never launched and has long been abandoned, and the original author doesn't think it's worth reviving. Time to get rid of it.

Code location:
/chrome/browser/thumbnails/content_based_thumbnailing_algorithm.h/cc
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 23 2017 

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

commit 1169f4bc4d22ad68c93c43be1fe32e4b6bc727b1
Author: Marc Treib <treib@chromium.org>
Date: Fri Jun 23 09:20:58 2017

Cleanup: Remove thumbnail retargeting code

This never launched and has long been abandoned.
This CL removes the code. Followups will remove all the wiring and
indirection in ThumbnailTabHelper and ThumbnailService.

Bug:  735905 
Change-Id: Ic6d125ea5c5020dcf3f484487257f9a221e59868
Reviewed-on: https://chromium-review.googlesource.com/544960
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481834}
[modify] https://crrev.com/1169f4bc4d22ad68c93c43be1fe32e4b6bc727b1/chrome/browser/BUILD.gn
[delete] https://crrev.com/437702b594f4bb8d95088af5bc05c4cd82d4dfc1/chrome/browser/thumbnails/content_analysis.cc
[delete] https://crrev.com/437702b594f4bb8d95088af5bc05c4cd82d4dfc1/chrome/browser/thumbnails/content_analysis.h
[delete] https://crrev.com/437702b594f4bb8d95088af5bc05c4cd82d4dfc1/chrome/browser/thumbnails/content_analysis_unittest.cc
[delete] https://crrev.com/437702b594f4bb8d95088af5bc05c4cd82d4dfc1/chrome/browser/thumbnails/content_based_thumbnailing_algorithm.cc
[delete] https://crrev.com/437702b594f4bb8d95088af5bc05c4cd82d4dfc1/chrome/browser/thumbnails/content_based_thumbnailing_algorithm.h
[delete] https://crrev.com/437702b594f4bb8d95088af5bc05c4cd82d4dfc1/chrome/browser/thumbnails/content_based_thumbnailing_algorithm_unittest.cc
[modify] https://crrev.com/1169f4bc4d22ad68c93c43be1fe32e4b6bc727b1/chrome/browser/thumbnails/thumbnail_service.h
[modify] https://crrev.com/1169f4bc4d22ad68c93c43be1fe32e4b6bc727b1/chrome/browser/thumbnails/thumbnail_service_impl.cc
[modify] https://crrev.com/1169f4bc4d22ad68c93c43be1fe32e4b6bc727b1/chrome/browser/thumbnails/thumbnail_service_impl.h
[modify] https://crrev.com/1169f4bc4d22ad68c93c43be1fe32e4b6bc727b1/chrome/common/chrome_switches.cc
[modify] https://crrev.com/1169f4bc4d22ad68c93c43be1fe32e4b6bc727b1/chrome/common/chrome_switches.h
[modify] https://crrev.com/1169f4bc4d22ad68c93c43be1fe32e4b6bc727b1/chrome/test/BUILD.gn

Comment 2 by treib@chromium.org, Jun 23 2017

Status: Started (was: Assigned)
Summary: Cleanup: Remove dead thumbnail code (was: Cleanup: Remove "thumbnail retargeting" code)
Turns out there's dead code in other thumbnails stuff too. Let's get it all cleaned up while I'm here.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 28 2017 

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

commit 44917233f4d723c182ec8f489f2bf488d7e42eec
Author: Marc Treib <treib@chromium.org>
Date: Wed Jun 28 16:55:59 2017

Cleanup: Remove dead code in SimpleThumbnailCrop

Also add some TODOs for more dead/pointless code.

Bug:  735905 
Change-Id: I658e1fff285966b4a31e6b62c2d4c68725387f88
Reviewed-on: https://chromium-review.googlesource.com/545596
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483030}
[modify] https://crrev.com/44917233f4d723c182ec8f489f2bf488d7e42eec/chrome/browser/thumbnails/simple_thumbnail_crop.cc
[modify] https://crrev.com/44917233f4d723c182ec8f489f2bf488d7e42eec/chrome/browser/thumbnails/simple_thumbnail_crop.h
[modify] https://crrev.com/44917233f4d723c182ec8f489f2bf488d7e42eec/chrome/browser/thumbnails/simple_thumbnail_crop_unittest.cc
Project Member

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

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

commit 8009f9df21b553ab113f86a512409656d6e86e17
Author: Marc Treib <treib@chromium.org>
Date: Mon Jul 03 09:32:48 2017

Thumbnails: Delete more dead code in SimpleThumbnailCrop

This removes some dead code for scaling the thumbnail to the desired
size. It's obsolete because the scaling happens during CopyFromSurface,
so the extra scaling is a no-op.

Bug:  735905 
Change-Id: Ic59cfb249d5481545b0957a93325ae1dedb07f8d
Reviewed-on: https://chromium-review.googlesource.com/557179
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483970}
[modify] https://crrev.com/8009f9df21b553ab113f86a512409656d6e86e17/chrome/browser/thumbnails/simple_thumbnail_crop.cc
[modify] https://crrev.com/8009f9df21b553ab113f86a512409656d6e86e17/chrome/browser/thumbnails/simple_thumbnail_crop.h
Project Member

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

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

commit e8cdd0f3addc397979cb2ed93ab06d5b4251ca76
Author: Marc Treib <treib@chromium.org>
Date: Mon Jul 03 10:39:44 2017

Thumbnails: Remove unnecessary plumbing

In particular, this removes ThumbnailingAlgorithm::ProcessBitmap, since
its only implementation doesn't actually do anything with the bitmap -
it just computes a score. That is moved directly into ThumbnailTabHelper.

Bug:  735905 
Change-Id: Icb8414bb9727fe9fdfbea711bdf469157f70c893
Reviewed-on: https://chromium-review.googlesource.com/557539
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483982}
[modify] https://crrev.com/e8cdd0f3addc397979cb2ed93ab06d5b4251ca76/chrome/browser/thumbnails/simple_thumbnail_crop.cc
[modify] https://crrev.com/e8cdd0f3addc397979cb2ed93ab06d5b4251ca76/chrome/browser/thumbnails/simple_thumbnail_crop.h
[modify] https://crrev.com/e8cdd0f3addc397979cb2ed93ab06d5b4251ca76/chrome/browser/thumbnails/thumbnail_service_impl.cc
[modify] https://crrev.com/e8cdd0f3addc397979cb2ed93ab06d5b4251ca76/chrome/browser/thumbnails/thumbnail_tab_helper.cc
[modify] https://crrev.com/e8cdd0f3addc397979cb2ed93ab06d5b4251ca76/chrome/browser/thumbnails/thumbnail_tab_helper.h
[modify] https://crrev.com/e8cdd0f3addc397979cb2ed93ab06d5b4251ca76/chrome/browser/thumbnails/thumbnailing_algorithm.h
[modify] https://crrev.com/e8cdd0f3addc397979cb2ed93ab06d5b4251ca76/chrome/browser/thumbnails/thumbnailing_context.cc
[modify] https://crrev.com/e8cdd0f3addc397979cb2ed93ab06d5b4251ca76/chrome/browser/thumbnails/thumbnailing_context.h
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 6 2017 

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

commit ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9
Author: Marc Treib <treib@chromium.org>
Date: Thu Jul 06 15:27:59 2017

Thumbnails: Remove ThumbnailingAlgorithm interface

It only had a single implementation, SimpleThumbnailCrop, and only a
single method, GetCanvasCopyInfo. SimpleThumbnailCrop::GetCanvasCopyInfo
doesn't really need any instance info; it's now a global function which
takes the one previous instance variable target_size_ as a parameter.

Bug:  735905 
Change-Id: Iede5d82cb5c56e18e311c734845b5acb15904a29
Reviewed-on: https://chromium-review.googlesource.com/558286
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484602}
[modify] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/browser/BUILD.gn
[delete] https://crrev.com/a3ebd9d7dbc3e6c716ce0b454c087a9a221f44a2/chrome/browser/thumbnails/simple_thumbnail_crop.h
[modify] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/browser/thumbnails/thumbnail_service.h
[modify] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/browser/thumbnails/thumbnail_service_impl.cc
[modify] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/browser/thumbnails/thumbnail_service_impl.h
[modify] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/browser/thumbnails/thumbnail_tab_helper.cc
[rename] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/browser/thumbnails/thumbnail_utils.cc
[add] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/browser/thumbnails/thumbnail_utils.h
[rename] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/browser/thumbnails/thumbnail_utils_unittest.cc
[delete] https://crrev.com/a3ebd9d7dbc3e6c716ce0b454c087a9a221f44a2/chrome/browser/thumbnails/thumbnailing_algorithm.h
[modify] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/browser/thumbnails/thumbnailing_context.cc
[modify] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/browser/thumbnails/thumbnailing_context.h
[modify] https://crrev.com/ca13ce668a72df97d03286c7e7d9e8a4e7c79ec9/chrome/test/BUILD.gn

Comment 7 by treib@chromium.org, Jul 7 2017

Status: Fixed (was: Started)
I think this can be considered done.
►

Sign in to add a comment