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

Issue 659955 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Feature

Blocking:
issue 660123



Sign in to add a comment

Move chrome://popular-sites-internals into components

Project Member Reported by sfiera@chromium.org, Oct 27 2016

Issue description

It can't be moved entirely, because of dependencies on //content, but the resources and the bulk of the logic can be moved.

This will allow Android and iOS to share the handler.
 

Comment 1 by fi...@chromium.org, Oct 27 2016

Labels: zine-popularsites M-56 zine-triaged
Status: Assigned (was: Untriaged)
Will this be part of M-56? If not please move to M-57.

Comment 2 by sfiera@chromium.org, Oct 27 2016

Labels: Type-Feature
Status: Started (was: Assigned)
M56. Review should be out shortly.

Comment 3 by sfiera@chromium.org, Oct 31 2016

Labels: zine-16-10-24 zine-16-10-31
Blocking: 660123
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 2 2016

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

commit 7713f244a63217c94cef663b5dbd153d00e7cdb6
Author: sfiera <sfiera@chromium.org>
Date: Wed Nov 02 17:09:38 2016

Add chrome://popular-sites-internals/ to iOS.

Move the resources and the bulk of the implementation into
//components/ntp_tiles. Leave behind the glue code for //content and add
the corresponding glue for iOS.

Some notes:
  * I couldn't figure out how to use gzipped resources for iOS, so I had
    to disable gzip on the resources.
  * The HTML is now flattened; this handles the <if/> tags. Apparently
    they were passed through to the browser before, which renders them
    ineffective.
  * The handler is not enabled on iOS yet; that's downstream.

This amounts to a ~3K blowup on Android (it was probably ~2K
compressed and ~5K now). There's probably some extra savings because I
think grit minifies the JS.

BUG= 659955 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/chrome/browser/android/ntp/most_visited_sites_bridge.cc
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/chrome/browser/android/ntp/popular_sites.cc
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/chrome/browser/android/ntp/popular_sites.h
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/chrome/browser/browser_resources.grd
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/chrome/browser/ui/BUILD.gn
[delete] https://crrev.com/b1f725f669f44add26c3b5dbc64c57ab18dec531/chrome/browser/ui/webui/popular_sites_internals_message_handler.h
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/chrome/browser/ui/webui/popular_sites_internals_ui.cc
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/ntp_tiles/BUILD.gn
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/ntp_tiles/DEPS
[rename] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/ntp_tiles/webui/popular_sites_internals_message_handler.cc
[add] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/ntp_tiles/webui/popular_sites_internals_message_handler.h
[add] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/ntp_tiles/webui/popular_sites_internals_message_handler_client.h
[rename] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/ntp_tiles/webui/resources/popular_sites_internals.css
[rename] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/ntp_tiles/webui/resources/popular_sites_internals.html
[rename] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/ntp_tiles/webui/resources/popular_sites_internals.js
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/resources/OWNERS
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/resources/components_resources.grd
[add] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/components/resources/ntp_tiles_resources.grdp
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/ios/chrome/browser/chrome_url_constants.cc
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/ios/chrome/browser/chrome_url_constants.h
[modify] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/ios/chrome/browser/ui/webui/BUILD.gn
[add] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/ios/chrome/browser/ui/webui/popular_sites_internals_ui.cc
[add] https://crrev.com/7713f244a63217c94cef663b5dbd153d00e7cdb6/ios/chrome/browser/ui/webui/popular_sites_internals_ui.h

So the remaining work here is the downstream implementation of the handler? Are you the right owner?
This is written and approved, but can't yet be submitted. It's been a rough week for bling's bots and there hasn't been a roll since the upstream half was submitted.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 7 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/25915e19adac0acb6d9e56b77bf063ec8189e9a4

commit 25915e19adac0acb6d9e56b77bf063ec8189e9a4
Author: sfiera <sfiera@google.com>
Date: Mon Nov 07 12:06:51 2016

Status: Fixed (was: Started)

Sign in to add a comment