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

Issue 754260 link

Starred by 1 user
Status: Fixed
Owner:
vitaliii@chromium.org
Closed: Sep 2017
Cc:
vitaliii@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

chrome://snippets-internals lies about the state of suggestions providers

Project Member Reported by bauerb@chromium.org, Aug 10 2017 
Steps to reproduce:
1) Enable Chrome Home
2) Go to chrome://snippets-internals
3) Look for "Bookmark Suggestions enabled", "Asset Download suggestions enabled", or "Offline Page Download Suggestions enabled"

Expected:
The providers should appear as disabled.

Actual:
chrome://snippets-internals claims they are enabled.

Comment 1 by vitaliii@chromium.org, Aug 10 2017 

Basically snippets-internals just looks at the corresponding features, while in the factory there is an additional Chrome Home check before checking the features itself.

I could take this after M62 FF.

Comment 2 by mvanouwe...@chromium.org, Aug 14 2017

Labels: M-62 zine-triaged
Owner: vitaliii@chromium.org
Status: Assigned (was: Untriaged)
Assigning to Vitalii and M62 as a bug fix, so it's ok after feature freeze.

Comment 3 by fi...@chromium.org, Sep 18 2017 

ping - I guess we missed it. New target M-63?

Comment 4 by vitaliii@chromium.org, Sep 25 2017

Labels: -M-62 M-63

Comment 5 by vitaliii@chromium.org, Sep 25 2017

Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 26 2017 

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

commit cdf7fe6d3826567eb2e01991174b137ef445b6ed
Author: Vitalii Iarko <vitaliii@chromium.org>
Date: Tue Sep 26 10:42:10 2017

[NTP] Move dependent features helpers to a separate file.

Due to Chrome Home, we have multiple features, which are enabled
conditionally on Chrome Home. This was achieved by having helpers
checking both features. However, these helpers were not available
outside of the factory.

In this CL:

1) Move these helpers into a separate file |dependent_features.{h,cc}|

2) Use the moved helpers in snippets_internals

3) Disable ForeignSessionsProvider in Chrome Home

4) Add a comment to components/ntp_snippets/features.h to not depend
on these features directly.

Bug:  754260 
Change-Id: Idf91f8a934c94048b0de1be459fe6004396e12a8
Reviewed-on: https://chromium-review.googlesource.com/681515
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Commit-Queue: vitaliii <vitaliii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504328}
[modify] https://crrev.com/cdf7fe6d3826567eb2e01991174b137ef445b6ed/chrome/browser/BUILD.gn
[modify] https://crrev.com/cdf7fe6d3826567eb2e01991174b137ef445b6ed/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
[add] https://crrev.com/cdf7fe6d3826567eb2e01991174b137ef445b6ed/chrome/browser/ntp_snippets/dependent_features.cc
[add] https://crrev.com/cdf7fe6d3826567eb2e01991174b137ef445b6ed/chrome/browser/ntp_snippets/dependent_features.h
[modify] https://crrev.com/cdf7fe6d3826567eb2e01991174b137ef445b6ed/chrome/browser/ui/webui/snippets_internals_message_handler.cc
[modify] https://crrev.com/cdf7fe6d3826567eb2e01991174b137ef445b6ed/chrome/common/chrome_features.cc
[modify] https://crrev.com/cdf7fe6d3826567eb2e01991174b137ef445b6ed/components/ntp_snippets/features.h

Comment 7 by vitaliii@chromium.org, Sep 26 2017

Status: Fixed (was: Started)
►

Sign in to add a comment