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

Issue 740115 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Not on Chrome anymore
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Task

Blocked on:
issue 706385



Sign in to add a comment

Remove unused suggestions preferences screen

Project Member Reported by dgn@chromium.org, Jul 7 2017

Issue description

We decided to use the suggestions toggle under privacy to also control content suggestions, so the preferences screen added in issue 706385 is currently unused.

The related code should also removed to allow reusing the method names. For example we want to expose whether remote suggestions are enabled, but the implementation will be very different from the current one.

Note: issue 706385 also involves adding a preference for the notifications. This one should stay as we need to surface it soon.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 10 2017

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

commit 02481b2984dd77002228c0f1aecadb220d5143b9
Author: Nicolas Dossou-gbete <dgn@chromium.org>
Date: Mon Jul 10 17:15:30 2017

[Suggestions] Surface correct AreRemoteSuggestionsEnabled value

The repo contains a currently unused preferences screen for the
suggestions, that is the only caller of the remote suggestion
status methods. This CL marks them as appropriately unreached.

The status of the remote suggestions will be used soon to update
the UI state, so AreRemoteSuggestionsEnabled is updated in this
patch to reflect the accurate status.

Bug:  721407 , 740115 
Change-Id: Ia92d85798f96064acc77c9366dafc3daee4ce025
Reviewed-on: https://chromium-review.googlesource.com/563617
Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485304}
[modify] https://crrev.com/02481b2984dd77002228c0f1aecadb220d5143b9/components/ntp_snippets/content_suggestions_service.cc
[modify] https://crrev.com/02481b2984dd77002228c0f1aecadb220d5143b9/components/ntp_snippets/remote/remote_suggestions_provider.h
[modify] https://crrev.com/02481b2984dd77002228c0f1aecadb220d5143b9/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
[modify] https://crrev.com/02481b2984dd77002228c0f1aecadb220d5143b9/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
[modify] https://crrev.com/02481b2984dd77002228c0f1aecadb220d5143b9/components/ntp_snippets/remote/remote_suggestions_scheduler_impl_unittest.cc

Comment 2 by fi...@chromium.org, Jul 20 2017

Labels: -Type-Bug zine-triaged Type-Task
Labels: -M-61 M-62

Comment 4 by dgn@chromium.org, Aug 25 2017

Strings related to this screen have already been removed in various CLs:

Add a notifications settings page for pre-O devices. - crrev.com/c/589848
Android: Remove UnusedResource suppressions - crrev.com/c/611091
Android: Remove unused resources - crrev.com/c/516362

CL cleaning up the pref class and unused JNI piping coming up.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 25 2017

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

commit bebf66f8f049e12a8a5af54adef9d79bbcd0209f
Author: Nicolas Dossou-gbete <dgn@chromium.org>
Date: Fri Aug 25 13:11:47 2017

[Suggestions] Remove leftovers of ContentSuggestionsPreferences

We don't have plans for suggestions preferences for M62, so
removing this old and dead code. It can be dug up later if needed,
rather than staying alongside used code.

Finishes the revert of https://crrev.com/b8a8a5c3a7ad

Bug:  740115 
Change-Id: I1c247c7eb0b4aa39a1f878a47705e8f727db0d90
Reviewed-on: https://chromium-review.googlesource.com/635344
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497386}
[modify] https://crrev.com/bebf66f8f049e12a8a5af54adef9d79bbcd0209f/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
[delete] https://crrev.com/9dc959dc041807f42424c3ba3da1d199d919cd8e/chrome/android/java/src/org/chromium/chrome/browser/preferences/ContentSuggestionsPreferences.java
[modify] https://crrev.com/bebf66f8f049e12a8a5af54adef9d79bbcd0209f/chrome/android/java/src/org/chromium/chrome/browser/preferences/MainPreferences.java
[modify] https://crrev.com/bebf66f8f049e12a8a5af54adef9d79bbcd0209f/chrome/browser/android/ntp/ntp_snippets_bridge.cc
[modify] https://crrev.com/bebf66f8f049e12a8a5af54adef9d79bbcd0209f/components/ntp_snippets/content_suggestions_service.cc
[modify] https://crrev.com/bebf66f8f049e12a8a5af54adef9d79bbcd0209f/components/ntp_snippets/content_suggestions_service.h

Comment 6 by dgn@chromium.org, Aug 25 2017

Status: Fixed (was: Assigned)

Sign in to add a comment