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

Issue 806251 link

Starred by 2 users
Status: Fixed
Owner:
█ ramyan@chromium.org
OOO Jan 14 - 25
Closed: Feb 2018
Cc:
ma...@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

NTP: Avoid expensive decoding of theme background image

Project Member Reported by treib@chromium.org, Jan 26 2018 
InstantService::BuildThemeInfo accesses the theme background image (IDR_THEME_NTP_BACKGROUND) only to get the image height. This causes the image to get decoded, which is fairly expensive, so the whole thing seems quite wasteful.

The image height is passed to the NTP (the actual JavaScript) through the embeddedSearch API as embeddedSearch.newTabPage.themeBackgroundInfo.imageHeight, which seems to be completely unused, and it's also not documented at https://www.chromium.org/embeddedsearch. So maybe the imageHeight field can just be removed, making the whole image access unnecessary.

See also internal bug b/70795531.

Comment 1 by treib@chromium.org, Jan 26 2018 

Note: If we remove that field, we should make sure that we don't break existing third-party NTP implementations (Bing and Yandex are the only ones AFAIK). I *think* those don't implement theming at all, but we should double-check.

Comment 2 by wittman@chromium.org, Jan 26 2018

Labels: UMA-Sampling-Profiler

Comment 3 by zea@chromium.org, Jan 30 2018

Labels: zine-triaged

Comment 4 by ramyan@chromium.org, Feb 1 2018

Cc: ma...@chromium.org
Owner: ramyan@chromium.org

Comment 5 by ramyan@chromium.org, Feb 1 2018

Status: Assigned (was: Available)

Comment 6 by ramyan@chromium.org, Feb 5 2018

Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 5 2018 

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

commit ee5a883e7a082e1456108378340c8013b330588d
Author: Ramya Nagarajan <ramyan@chromium.org>
Date: Mon Feb 05 22:23:44 2018

Removed ThemeBackgroundInfo.image_height.

The theme background image is accessed (and decoded) by InstantService::BuildThemeInfo only to get the image height. It is then passed to the NTP via the embeddedSearch API, where it is unused. Removing this unnecessary access should improve NTP load time in some instances.

Bug:  806251 
Change-Id: I8f15c51e4b2ac9b9cbd880ac7586c7c92b4e0416
Reviewed-on: https://chromium-review.googlesource.com/902184
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Ramya Nagarajan <ramyan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534506}
[modify] https://crrev.com/ee5a883e7a082e1456108378340c8013b330588d/chrome/browser/search/instant_service.cc
[modify] https://crrev.com/ee5a883e7a082e1456108378340c8013b330588d/chrome/common/instant_struct_traits.h
[modify] https://crrev.com/ee5a883e7a082e1456108378340c8013b330588d/chrome/common/search/instant_types.cc
[modify] https://crrev.com/ee5a883e7a082e1456108378340c8013b330588d/chrome/common/search/instant_types.h
[modify] https://crrev.com/ee5a883e7a082e1456108378340c8013b330588d/chrome/renderer/searchbox/searchbox_extension.cc

Comment 8 by ramyan@chromium.org, Feb 5 2018

Status: Fixed (was: Started)
►

Sign in to add a comment