Skip to content

Commit

Permalink
Rename discard to dismiss for NTP snippets and content suggestions
Browse files Browse the repository at this point in the history
We agreed to use the term 'dismiss' instead of 'discard' for the action
performed by the user to remove content suggestions from the NTP.
These changes are only for the non-Android part. The renaming for the
Android part will follow after or during the ongoing refactoring there.

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

Review-Url: https://codereview.chromium.org/2167063003
Cr-Commit-Position: refs/heads/master@{#407456}
  • Loading branch information
pke authored and Commit bot committed Jul 25, 2016
1 parent 1bd8b75 commit 2646c95
Show file tree
Hide file tree
Showing 18 changed files with 131 additions and 131 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/android/ntp/ntp_snippets_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ void NTPSnippetsBridge::FetchImage(JNIEnv* env,
void NTPSnippetsBridge::DiscardSnippet(JNIEnv* env,
const JavaParamRef<jobject>& obj,
const JavaParamRef<jstring>& id) {
content_suggestions_service_->DiscardSuggestion(
content_suggestions_service_->DismissSuggestion(
ConvertJavaStringToUTF8(env, id));
}

Expand Down
14 changes: 7 additions & 7 deletions chrome/browser/resources/snippets_internals.html
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,12 @@ <h2>Last JSON</h2>
</div>
</div>

<div id="discarded-snippets">
<h2>Discarded snippets <span class="detail">(click for details)</span></h2>
<div id="dismissed-snippets">
<h2>Dismissed snippets <span class="detail">(click for details)</span></h2>
<table class="section-details">
<tr jsselect="list" style="display:none">
<td class="title-link">
<span class="discarded-snippet-title" jsvalues="hidden-id:id">
<span class="dismissed-snippet-title" jsvalues="hidden-id:id">
<span jscontent="title"></span> &gt;&gt;</span>
<div jsvalues="id:id" class="snippet-detail hidden">
<table>
Expand Down Expand Up @@ -140,9 +140,9 @@ <h2>Discarded snippets <span class="detail">(click for details)</span></h2>
</table>
</div>
</table>
<div class="detail" id="discarded-snippets-empty"></div>
<div class="detail" id="dismissed-snippets-empty"></div>
<div class="forms">
<input id="discarded-snippets-clear" type="submit" value="Clear list">
<input id="dismissed-snippets-clear" type="submit" value="Clear list">
</div>
</div>

Expand Down Expand Up @@ -205,8 +205,8 @@ <h3>
<div class="forms">
<input id="submit-clear-cached-suggestions" type="submit"
value="Clear cached suggestions">
<input id="submit-clear-discarded-suggestions" type="submit"
value="Clear discarded suggestions">
<input id="submit-clear-dismissed-suggestions" type="submit"
value="Clear dismissed suggestions">
</div>
</div>
</div>
16 changes: 8 additions & 8 deletions chrome/browser/resources/snippets_internals.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ cr.define('chrome.SnippetsInternals', function() {
event.preventDefault();
});

$('discarded-snippets-clear').addEventListener('click', function(event) {
chrome.send('clearDiscarded');
$('dismissed-snippets-clear').addEventListener('click', function(event) {
chrome.send('clearDismissed');
event.preventDefault();
});

Expand All @@ -44,9 +44,9 @@ cr.define('chrome.SnippetsInternals', function() {
event.preventDefault();
});

$('submit-clear-discarded-suggestions')
$('submit-clear-dismissed-suggestions')
.addEventListener('click', function(event) {
chrome.send('clearDiscardedSuggestions');
chrome.send('clearDismissedSuggestions');
event.preventDefault();
});

Expand Down Expand Up @@ -79,9 +79,9 @@ cr.define('chrome.SnippetsInternals', function() {
displayList(snippets, 'snippets', 'snippet-title');
}

function receiveDiscardedSnippets(discardedSnippets) {
displayList(discardedSnippets, 'discarded-snippets',
'discarded-snippet-title');
function receiveDismissedSnippets(dismissedSnippets) {
displayList(dismissedSnippets, 'dismissed-snippets',
'dismissed-snippet-title');
}

function receiveContentSuggestions(categoriesList) {
Expand Down Expand Up @@ -150,7 +150,7 @@ cr.define('chrome.SnippetsInternals', function() {
receiveProperty: receiveProperty,
receiveHosts: receiveHosts,
receiveSnippets: receiveSnippets,
receiveDiscardedSnippets: receiveDiscardedSnippets,
receiveDismissedSnippets: receiveDismissedSnippets,
receiveContentSuggestions: receiveContentSuggestions,
receiveJson: receiveJson,
};
Expand Down
32 changes: 16 additions & 16 deletions chrome/browser/ui/webui/snippets_internals_message_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace {
std::unique_ptr<base::DictionaryValue> PrepareSnippet(
const ntp_snippets::NTPSnippet& snippet,
int index,
bool discarded) {
bool dismissed) {
std::unique_ptr<base::DictionaryValue> entry(new base::DictionaryValue);
entry->SetString("snippetId", snippet.id());
entry->SetString("title", snippet.title());
Expand All @@ -50,8 +50,8 @@ std::unique_ptr<base::DictionaryValue> PrepareSnippet(
entry->SetString("salientImageUrl", snippet.salient_image_url().spec());
entry->SetDouble("score", snippet.score());

if (discarded)
entry->SetString("id", "discarded-snippet-" + base::IntToString(index));
if (dismissed)
entry->SetString("id", "dismissed-snippet-" + base::IntToString(index));
else
entry->SetString("id", "snippet-" + base::IntToString(index));

Expand Down Expand Up @@ -183,8 +183,8 @@ void SnippetsInternalsMessageHandler::RegisterMessages() {
base::Unretained(this)));

web_ui()->RegisterMessageCallback(
"clearDiscarded",
base::Bind(&SnippetsInternalsMessageHandler::HandleClearDiscarded,
"clearDismissed",
base::Bind(&SnippetsInternalsMessageHandler::HandleClearDismissed,
base::Unretained(this)));

web_ui()->RegisterMessageCallback(
Expand All @@ -193,9 +193,9 @@ void SnippetsInternalsMessageHandler::RegisterMessages() {
base::Unretained(this)));

web_ui()->RegisterMessageCallback(
"clearDiscardedSuggestions",
"clearDismissedSuggestions",
base::Bind(
&SnippetsInternalsMessageHandler::HandleClearDiscardedSuggestions,
&SnippetsInternalsMessageHandler::HandleClearDismissedSuggestions,
base::Unretained(this)));
}

Expand All @@ -214,12 +214,12 @@ void SnippetsInternalsMessageHandler::HandleClear(const base::ListValue* args) {
ntp_snippets_service_->ClearCachedSuggestionsForDebugging();
}

void SnippetsInternalsMessageHandler::HandleClearDiscarded(
void SnippetsInternalsMessageHandler::HandleClearDismissed(
const base::ListValue* args) {
DCHECK_EQ(0u, args->GetSize());

ntp_snippets_service_->ClearDiscardedSuggestionsForDebugging();
SendDiscardedSnippets();
ntp_snippets_service_->ClearDismissedSuggestionsForDebugging();
SendDismissedSnippets();
}

void SnippetsInternalsMessageHandler::HandleDownload(
Expand All @@ -245,11 +245,11 @@ void SnippetsInternalsMessageHandler::HandleClearCachedSuggestions(
content_suggestions_service_->ClearCachedSuggestionsForDebugging();
}

void SnippetsInternalsMessageHandler::HandleClearDiscardedSuggestions(
void SnippetsInternalsMessageHandler::HandleClearDismissedSuggestions(
const base::ListValue* args) {
DCHECK_EQ(0u, args->GetSize());

content_suggestions_service_->ClearDiscardedSuggestionsForDebugging();
content_suggestions_service_->ClearDismissedSuggestionsForDebugging();
}

void SnippetsInternalsMessageHandler::SendAllContent() {
Expand Down Expand Up @@ -284,7 +284,7 @@ void SnippetsInternalsMessageHandler::SendAllContent() {
ntp_snippets_service_->snippets_fetcher()->fetch_url().spec());

SendSnippets();
SendDiscardedSnippets();
SendDismissedSnippets();
SendContentSuggestions();
}

Expand All @@ -307,17 +307,17 @@ void SnippetsInternalsMessageHandler::SendSnippets() {
SendString("hosts-status", "Finished: " + status);
}

void SnippetsInternalsMessageHandler::SendDiscardedSnippets() {
void SnippetsInternalsMessageHandler::SendDismissedSnippets() {
std::unique_ptr<base::ListValue> snippets_list(new base::ListValue);

int index = 0;
for (const auto& snippet : ntp_snippets_service_->discarded_snippets())
for (const auto& snippet : ntp_snippets_service_->dismissed_snippets())
snippets_list->Append(PrepareSnippet(*snippet, index++, true));

base::DictionaryValue result;
result.Set("list", std::move(snippets_list));
web_ui()->CallJavascriptFunctionUnsafe(
"chrome.SnippetsInternals.receiveDiscardedSnippets", result);
"chrome.SnippetsInternals.receiveDismissedSnippets", result);
}

void SnippetsInternalsMessageHandler::SendHosts() {
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/ui/webui/snippets_internals_message_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ class SnippetsInternalsMessageHandler

void HandleRefreshContent(const base::ListValue* args);
void HandleClear(const base::ListValue* args);
void HandleClearDiscarded(const base::ListValue* args);
void HandleClearDismissed(const base::ListValue* args);
void HandleDownload(const base::ListValue* args);
void HandleClearCachedSuggestions(const base::ListValue* args);
void HandleClearDiscardedSuggestions(const base::ListValue* args);
void HandleClearDismissedSuggestions(const base::ListValue* args);

void SendAllContent();
void SendSnippets();
void SendDiscardedSnippets();
void SendDismissedSnippets();
void SendHosts();
void SendContentSuggestions();
void SendBoolean(const std::string& name, bool value);
Expand Down
14 changes: 7 additions & 7 deletions components/ntp_snippets/content_suggestions_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ class ContentSuggestionsProvider {
virtual ContentSuggestionsCategoryStatus GetCategoryStatus(
ContentSuggestionsCategory category) = 0;

// Discards the suggestion with the given ID. A provider needs to ensure that
// a once-discarded suggestion is never delivered again (through the
// Dismisses the suggestion with the given ID. A provider needs to ensure that
// a once-dismissed suggestion is never delivered again (through the
// Observer). The provider must not call Observer::OnSuggestionsChanged if the
// removal of the discarded suggestion is the only change.
virtual void DiscardSuggestion(const std::string& suggestion_id) = 0;
// removal of the dismissed suggestion is the only change.
virtual void DismissSuggestion(const std::string& suggestion_id) = 0;

// Fetches the image for the suggestion with the given ID and returns it
// through the callback. This fetch may occur locally or from the internet.
Expand All @@ -88,11 +88,11 @@ class ContentSuggestionsProvider {
// fetch starts from scratch.
virtual void ClearCachedSuggestionsForDebugging() = 0;

// Used only for debugging purposes. Clears the cache of discarded
// Used only for debugging purposes. Clears the cache of dismissed
// suggestions, if present, so that no suggestions are suppressed. This does
// not necessarily make previously discarded suggestions reappear, as they may
// not necessarily make previously dismissed suggestions reappear, as they may
// have been permanently deleted, depending on the provider implementation.
virtual void ClearDiscardedSuggestionsForDebugging() = 0;
virtual void ClearDismissedSuggestionsForDebugging() = 0;

const std::vector<ContentSuggestionsCategory>& provided_categories() const {
return provided_categories_;
Expand Down
16 changes: 8 additions & 8 deletions components/ntp_snippets/content_suggestions_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,25 +77,25 @@ void ContentSuggestionsService::ClearCachedSuggestionsForDebugging() {
FOR_EACH_OBSERVER(Observer, observers_, OnNewSuggestions());
}

void ContentSuggestionsService::ClearDiscardedSuggestionsForDebugging() {
void ContentSuggestionsService::ClearDismissedSuggestionsForDebugging() {
for (auto& category_provider_pair : providers_) {
category_provider_pair.second->ClearDiscardedSuggestionsForDebugging();
category_provider_pair.second->ClearDismissedSuggestionsForDebugging();
}
}

void ContentSuggestionsService::DiscardSuggestion(
void ContentSuggestionsService::DismissSuggestion(
const std::string& suggestion_id) {
if (!id_category_map_.count(suggestion_id)) {
LOG(WARNING) << "Discarded unknown suggestion " << suggestion_id;
LOG(WARNING) << "Dismissed unknown suggestion " << suggestion_id;
return;
}
ContentSuggestionsCategory category = id_category_map_[suggestion_id];
if (!providers_.count(category)) {
LOG(WARNING) << "Discarded suggestion " << suggestion_id
LOG(WARNING) << "Dismissed suggestion " << suggestion_id
<< " for unavailable category " << static_cast<int>(category);
return;
}
providers_[category]->DiscardSuggestion(suggestion_id);
providers_[category]->DismissSuggestion(suggestion_id);

// Remove the suggestion locally.
id_category_map_.erase(suggestion_id);
Expand All @@ -107,9 +107,9 @@ void ContentSuggestionsService::DiscardSuggestion(
return suggestion_id == suggestion.id();
});
DCHECK(position != suggestions->end())
<< "The discarded suggestion " << suggestion_id
<< "The dismissed suggestion " << suggestion_id
<< " has already been removed. Providers must not call OnNewSuggestions"
" in response to DiscardSuggestion.";
" in response to DismissSuggestion.";
suggestions->erase(position);
}

Expand Down
12 changes: 6 additions & 6 deletions components/ntp_snippets/content_suggestions_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ class ContentSuggestionsService : public KeyedService,
void FetchSuggestionImage(const std::string& suggestion_id,
const ImageFetchedCallback& callback);

// Discards the suggestion with the given |suggestion_id|, if it exists.
// Dismisses the suggestion with the given |suggestion_id|, if it exists.
// This will not trigger an update through the observers.
void DiscardSuggestion(const std::string& suggestion_id);
void DismissSuggestion(const std::string& suggestion_id);

// Observer accessors.
void AddObserver(Observer* observer);
Expand All @@ -110,14 +110,14 @@ class ContentSuggestionsService : public KeyedService,
// providers. It does, however, not remove any suggestions from the provider's
// sources, so if their configuration hasn't changed, they should return the
// same results when they fetch the next time. In particular, calling this
// method will not mark any suggestions as discarded.
// method will not mark any suggestions as dismissed.
void ClearCachedSuggestionsForDebugging();

// Only for debugging use through the internals page. Some providers
// internally store a list of discarded suggestions to prevent them from
// internally store a list of dismissed suggestions to prevent them from
// reappearing. This function clears all such lists in all providers, making
// discarded suggestions reappear (only for certain providers).
void ClearDiscardedSuggestionsForDebugging();
// dismissed suggestions reappear (only for certain providers).
void ClearDismissedSuggestionsForDebugging();

private:
friend class ContentSuggestionsServiceTest;
Expand Down
12 changes: 6 additions & 6 deletions components/ntp_snippets/content_suggestions_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ class MockProvider : public ContentSuggestionsProvider {
}

MOCK_METHOD0(ClearCachedSuggestionsForDebugging, void());
MOCK_METHOD0(ClearDiscardedSuggestionsForDebugging, void());
MOCK_METHOD1(DiscardSuggestion, void(const std::string& suggestion_id));
MOCK_METHOD0(ClearDismissedSuggestionsForDebugging, void());
MOCK_METHOD1(DismissSuggestion, void(const std::string& suggestion_id));
MOCK_METHOD2(FetchSuggestionImage,
void(const std::string& suggestion_id,
const ImageFetchedCallback& callback));
Expand Down Expand Up @@ -305,7 +305,7 @@ TEST_F(ContentSuggestionsServiceTest,
base::Unretained(this)));
}

TEST_F(ContentSuggestionsServiceTest, ShouldRedirectDiscardSuggestion) {
TEST_F(ContentSuggestionsServiceTest, ShouldRedirectDismissSuggestion) {
MockProvider provider1(ContentSuggestionsCategory::ARTICLES);
MockProvider provider2(ContentSuggestionsCategory::OFFLINE_PAGES);
service()->RegisterProvider(&provider1);
Expand All @@ -315,9 +315,9 @@ TEST_F(ContentSuggestionsServiceTest, ShouldRedirectDiscardSuggestion) {
{11});
std::string suggestion_id = CreateSuggestion(11).id();

EXPECT_CALL(provider1, DiscardSuggestion(_)).Times(0);
EXPECT_CALL(provider2, DiscardSuggestion(suggestion_id)).Times(1);
service()->DiscardSuggestion(suggestion_id);
EXPECT_CALL(provider1, DismissSuggestion(_)).Times(0);
EXPECT_CALL(provider2, DismissSuggestion(suggestion_id)).Times(1);
service()->DismissSuggestion(suggestion_id);
provider1.FireShutdown();
provider2.FireShutdown();
}
Expand Down
6 changes: 3 additions & 3 deletions components/ntp_snippets/ntp_snippet.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ bool GetURLValue(const base::DictionaryValue& dict,
namespace ntp_snippets {

NTPSnippet::NTPSnippet(const std::string& id)
: id_(id), score_(0), is_discarded_(false), best_source_index_(0) {}
: id_(id), score_(0), is_dismissed_(false), best_source_index_(0) {}

NTPSnippet::~NTPSnippet() {}

Expand Down Expand Up @@ -184,7 +184,7 @@ std::unique_ptr<NTPSnippet> NTPSnippet::CreateFromProto(
base::Time::FromInternalValue(proto.publish_date()));
snippet->set_expiry_date(base::Time::FromInternalValue(proto.expiry_date()));
snippet->set_score(proto.score());
snippet->set_discarded(proto.discarded());
snippet->set_dismissed(proto.dismissed());

for (int i = 0; i < proto.sources_size(); ++i) {
const SnippetSourceProto& source_proto = proto.sources(i);
Expand Down Expand Up @@ -230,7 +230,7 @@ SnippetProto NTPSnippet::ToProto() const {
if (!expiry_date_.is_null())
result.set_expiry_date(expiry_date_.ToInternalValue());
result.set_score(score_);
result.set_discarded(is_discarded_);
result.set_dismissed(is_dismissed_);

for (const SnippetSource& source : sources_) {
SnippetSourceProto* source_proto = result.add_sources();
Expand Down
6 changes: 3 additions & 3 deletions components/ntp_snippets/ntp_snippet.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ class NTPSnippet {
float score() const { return score_; }
void set_score(float score) { score_ = score; }

bool is_discarded() const { return is_discarded_; }
void set_discarded(bool discarded) { is_discarded_ = discarded; }
bool is_dismissed() const { return is_dismissed_; }
void set_dismissed(bool dismissed) { is_dismissed_ = dismissed; }

// Public for testing.
static base::Time TimeFromJsonString(const std::string& timestamp_str);
Expand All @@ -139,7 +139,7 @@ class NTPSnippet {
base::Time publish_date_;
base::Time expiry_date_;
float score_;
bool is_discarded_;
bool is_dismissed_;

size_t best_source_index_;

Expand Down
Loading

0 comments on commit 2646c95

Please sign in to comment.