Use multiple IDs when discarding or merging snippets.

components/ntp_snippets/ntp_snippet.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_NTP_SNIPPETS_NTP_SNIPPET_H_
 #define COMPONENTS_NTP_SNIPPETS_NTP_SNIPPET_H_
 
 #include <memory>
 #include <string>
 #include <vector>
@@ skipping 39 matching lines @@
   // |snippets|. Returns true on success, false if anything went wrong.
   static bool AddFromListValue(const base::ListValue& list,
                                PtrVector* snippets);
 
   std::unique_ptr<base::DictionaryValue> ToDictionary() const;
 
   // A unique ID for identifying the snippet. If initialized by
   // CreateFromDictionary() the relevant key is 'url'.
   // TODO(treib): For now, the ID has to be a valid URL spec, otherwise
   // fetching the salient image will fail. See TODO in ntp_snippets_service.cc.
+  // TODO(tschumann): With the new data layout based on the zine backend service
+  // protos, we should distinguish between an ID (should only be used by the JNI
+  // bridge) and a URL (used by snipets internal page etc.).

[Marc Treib, 2016/05/18 15:19:33]

Not quite sure how to read this comment... we already have an ID vs a set of
URLs, so what's the TODO? Also, I think we'd want to continue showing the ID on
the internals page?

nitty nit: snipPets internalS

[tschumann, 2016/05/18 17:09:16]

IMO we should properly distinguish the ID from URLs. In the  internals page, we
should only show a URL (and in most other places too). The id should be used
only to identify a snippet. 
Fixed the comment. 
If you feel this is comment is premature, I can also remove it =)

[Marc Treib, 2016/05/18 18:58:05]

IMO the internals page should show everything we have. That's what it's for. But
that's a discussion for another day :)
I kinda think it is, but I don't feel that strongly.
I wouldn't mention the JNI bridge though - if we have an ID, then it should be
an actual ID, without restrictions on who can use it...

[tschumann, 2016/05/18 19:23:28]

Removed the comment. As it's not too clear as of today :-)
(my reasoning was to keep the ID hidden as much as possible. If we need a string
to display in a UI, we can ask the NTPSnippet object to give us such a string,
but we shouldn't ask for implementation details. To be honest, the current
design of the class is more like a data type and doesn't hide any information 
-- need to read more code to judge if we should hide more or not. Especially
when integrating with other platforms, better data hiding would allow us more
flexibility).  

   const std::string& id() const { return id_; }
 
   // Title of the snippet.
   const std::string& title() const { return title_; }
   void set_title(const std::string& title) { title_ = title; }
 
   // Summary or relevant extract from the content.
   const std::string& snippet() const { return snippet_; }
   void set_snippet(const std::string& snippet) { snippet_ = snippet; }
 
@@ skipping 57 matching lines @@
   size_t best_source_index_;
 
   std::vector<SnippetSource> sources_;
 
   DISALLOW_COPY_AND_ASSIGN(NTPSnippet);
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_NTP_SNIPPET_H_