Scaffolding for an experimental discovery API letting users inject links in the recommended pane of…

chrome/browser/extensions/extension_suggested_links_manager.h

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[Aaron Boodman, 2012/05/11 21:06:46]

You should unit test this middle-level code.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_SUGGESTED_LINKS_MANAGER_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_SUGGESTED_LINKS_MANAGER_H_
+#pragma once
+
+#include <map>
+
+#include "base/memory/ref_counted.h"
+#include "chrome/browser/extensions/extension_suggested_link.h"
+#include "content/public/browser/browser_thread.h"
+
+// This class keeps track of links suggested by an extension using the discovery

[Aaron Boodman, 2012/05/11 21:06:46]

Are the suggestions not saved anywhere? I think they need to be. It's
inefficient to start the extension on every browser startup, just so that it can
suggest these links. It should only do it when it has new links to suggest.
Also, the way you have it means that the extension startup needs to block NTP,
which we don't want to do.

I think that the links should be saved in prefs. There should be a limit to how
many links can be suggested per-extension, like 3 or 5 to avoid bloating the
preferences.

If you make that change, you can get rid of this manager object with
profile-lifetime. Instead you can just have a little helper class that can read
and write from preferences and which is created when needed.

[beaudoin, 2012/05/12 03:16:29]

The expected usage is for the links to be dynamically generated with freshness
in mind, so for typical use cases links stored in preferences would be stale and
uninteresting. In fact, they may even be detrimental since, currently, the
NTP-recommended pane fills its spots with top links from history when no
extension can provide discovery links.

Regarding blocking the NTP on extension startup, I believe the solution should
be in the NTP logic, where it should not wait on extension startup. In fact, the
model I'm working on (for the next CL) already does this: using only
recommendations from currently started-up and running extensions.

[beaudoin, 2012/05/14 23:28:02]

Discussed in side thread.

+// API.
+class ExtensionSuggestedLinksManager
+    : public base::RefCountedThreadSafe<
+          ExtensionSuggestedLinksManager,
+          content::BrowserThread::DeleteOnUIThread> {
+ public:
+  ExtensionSuggestedLinksManager();
+
+  // Takes ownership of |item| in all cases.
+  void Add(const std::string& extension_id, ExtensionSuggestedLink* item);
+
+  const ExtensionSuggestedLinkList* GetAll(
+      const std::string& extension_id) const;
+
+  void Clear(const std::string& extension_id, const std::string& linkUrl);
+
+  // Clears all suggested links for |extension_id|.
+  void ClearAll(const std::string& extension_id);
+
+ private:
+  friend class base::DeleteHelper<ExtensionSuggestedLinksManager>;
+  friend struct content::BrowserThread::DeleteOnThread<
+      content::BrowserThread::UI>;
+
+  // Maps extension id to a list of notifications for that extension.
+  typedef std::map<std::string, ExtensionSuggestedLinkList> SuggestedLinksMap;
+
+  virtual ~ExtensionSuggestedLinksManager();
+
+  // Gets suggested links for a given extension id.
+  ExtensionSuggestedLinkList& GetAllInternal(const std::string& extension_id);
+
+  SuggestedLinksMap suggested_links_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionSuggestedLinksManager);
+};
+
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_SUGGESTED_LINKS_MANAGER_H_