Add support for distilling arbitrary URLs in DOM Distiller Viewer.

components/dom_distiller/core/dom_distiller_service.h

 // Copyright 2013 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_DOM_DISTILLER_CORE_DOM_DISTILLER_SERVICE_H_
 #define COMPONENTS_DOM_DISTILLER_CORE_DOM_DISTILLER_SERVICE_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 12 matching lines @@
 namespace dom_distiller {
 
 class DistilledArticleProto;
 class DistillerFactory;
 class DomDistillerObserver;
 class DomDistillerStoreInterface;
 class TaskTracker;
 class ViewerHandle;
 class ViewRequestDelegate;
 
-// Provide a view of the article list and ways of interacting with it.
-class DomDistillerService {
+// Service for interacting with the Dom Distiller.
+// Calls to, construction and destruction of this service must happen on the

[cjhopman, 2014/02/25 22:25:14]

This comma reads like a typo; i.e. that you meant "calls to construction and
destruction". How about "Construction, destruction, and usage of this ..."

[nyquist, 2014/02/27 00:07:37]

Done.

+// same thread. All callbacks will also be called on the caller's thread.

[cjhopman, 2014/02/25 22:25:14]

I'd say: Callbacks will be called on that same thread.

That is, refer back to the thread in the previous sentence rather than
introducing a new "thread"

[nyquist, 2014/02/27 00:07:37]

Done.

+class DomDistillerServiceInterface {
  public:
   typedef base::Callback<void(bool)> ArticleAvailableCallback;
+  virtual ~DomDistillerServiceInterface() {}
 
-  DomDistillerService(scoped_ptr<DomDistillerStoreInterface> store,
-                      scoped_ptr<DistillerFactory> distiller_factory);
-  ~DomDistillerService();
-
-  syncer::SyncableService* GetSyncableService() const;
+  virtual syncer::SyncableService* GetSyncableService() const = 0;
 
   // Distill the article at |url| and add the resulting entry to the DOM
   // distiller list. |article_cb| is invoked with true if article is
   // available offline.

[cjhopman, 2014/02/25 22:25:14]

Clarify the callback part of this: is it always called? is it called with false
if the article isn't available offline?

[nyquist, 2014/02/27 00:07:37]

Done.

-  const std::string AddToList(const GURL& url,
-                              const ArticleAvailableCallback& article_cb);
+  virtual const std::string AddToList(
+      const GURL& url,
+      const ArticleAvailableCallback& article_cb) = 0;
 
   // Gets the full list of entries.
-  std::vector<ArticleEntry> GetEntries() const;
+  virtual std::vector<ArticleEntry> GetEntries() const = 0;
 
   // Removes the specified entry from the dom distiller store.
-  scoped_ptr<ArticleEntry> RemoveEntry(const std::string& entry_id);
+  virtual scoped_ptr<ArticleEntry> RemoveEntry(const std::string& entry_id) = 0;
 
   // Request to view an article by entry id. Returns a null pointer if no entry
   // with |entry_id| exists. The ViewerHandle should be destroyed before the
   // ViewRequestDelegate. The request will be cancelled when the handle is
   // destroyed (or when this service is destroyed).

[cjhopman, 2014/02/25 22:25:14]

We should be clear about how we will handle the ViewRequestDelegate (either here
or in its declaration).

For example, is it possible to get a callback into the delegate after I've
deleted the handle? 

[nyquist, 2014/02/27 00:07:37]

Done.

-  scoped_ptr<ViewerHandle> ViewEntry(ViewRequestDelegate* delegate,
-                                     const std::string& entry_id);
+  virtual scoped_ptr<ViewerHandle> ViewEntry(ViewRequestDelegate* delegate,
+                                             const std::string& entry_id) = 0;
 
   // Request to view an article by url.
-  scoped_ptr<ViewerHandle> ViewUrl(ViewRequestDelegate* delegate,
-                                   const GURL& url);
+  virtual scoped_ptr<ViewerHandle> ViewUrl(ViewRequestDelegate* delegate,
+                                           const GURL& url) = 0;
 
-  void AddObserver(DomDistillerObserver* observer);
-  void RemoveObserver(DomDistillerObserver* observer);
+  virtual void AddObserver(DomDistillerObserver* observer) = 0;
+  virtual void RemoveObserver(DomDistillerObserver* observer) = 0;

[shashi, 2014/02/25 21:48:08]

Should the interface also have DISALLOW_COPY_AND_ASSIGN?

[nyquist, 2014/02/27 00:07:37]

Done.

+};
+
+// Provide a view of the article list and ways of interacting with it.
+class DomDistillerService : public DomDistillerServiceInterface {
+ public:
+  DomDistillerService(scoped_ptr<DomDistillerStoreInterface> store,
+                      scoped_ptr<DistillerFactory> distiller_factory);
+  virtual ~DomDistillerService();
+
+  // DomDistillerServiceInterface implementation.
+  virtual syncer::SyncableService* GetSyncableService() const OVERRIDE;
+  virtual const std::string AddToList(
+      const GURL& url,
+      const ArticleAvailableCallback& article_cb) OVERRIDE;
+  virtual std::vector<ArticleEntry> GetEntries() const OVERRIDE;
+  virtual scoped_ptr<ArticleEntry> RemoveEntry(const std::string& entry_id)
+      OVERRIDE;
+  virtual scoped_ptr<ViewerHandle> ViewEntry(ViewRequestDelegate* delegate,
+                                             const std::string& entry_id)
+      OVERRIDE;
+  virtual scoped_ptr<ViewerHandle> ViewUrl(ViewRequestDelegate* delegate,
+                                           const GURL& url) OVERRIDE;
+  virtual void AddObserver(DomDistillerObserver* observer) OVERRIDE;
+  virtual void RemoveObserver(DomDistillerObserver* observer) OVERRIDE;
 
  private:
   void CancelTask(TaskTracker* task);
   void AddDistilledPageToList(const ArticleEntry& entry,
                               const DistilledArticleProto* article_proto,
                               bool distillation_succeeded);
 
   TaskTracker* CreateTaskTracker(const ArticleEntry& entry);
 
   TaskTracker* GetTaskTrackerForEntry(const ArticleEntry& entry) const;
 
   // Gets the task tracker for the given |url| or |entry|. If no appropriate
   // tracker exists, this will create one, initialize it, and add it to
   // |tasks_|.
   TaskTracker* GetOrCreateTaskTrackerForUrl(const GURL& url);
   TaskTracker* GetOrCreateTaskTrackerForEntry(const ArticleEntry& entry);
 
   scoped_ptr<DomDistillerStoreInterface> store_;
   scoped_ptr<DistillerFactory> distiller_factory_;
 
   typedef ScopedVector<TaskTracker> TaskList;
   TaskList tasks_;
 
   DISALLOW_COPY_AND_ASSIGN(DomDistillerService);
 };
 
 }  // namespace dom_distiller
 
 #endif  // COMPONENTS_DOM_DISTILLER_CORE_DOM_DISTILLER_SERVICE_H_