Improve syncing between dom distiller and the print preview request

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 74 matching lines @@
 
   // Request to view an article by url.
   // Use CreateDefaultDistillerPage() to create a default |distiller_page|.
   // The provided |distiller_page| is only used if there is not already a
   // distillation task in progress for the given |url|.
   virtual scoped_ptr<ViewerHandle> ViewUrl(
       ViewRequestDelegate* delegate,
       scoped_ptr<DistillerPage> distiller_page,
       const GURL& url) = 0;
 
+  // Adds |delegate| to the provided |url| task tracker.
+  // If the given |url| has no task tracker (i.e. has no current
+  // distillation request underway), this will return nullptr - therefore
+  // this should only be called after previously requesting a distillation.
+  // If the inclusion of |delegate| on the |url| task tracker was
+  // successful, returns a |ViewerHandle| from the task tracker. This
+  // |ViewerHandle| can be held by client code as long as it wants to
+  // receive callbacks as a |ViewRequestDelegate|.

[nyquist, 2016/02/11 15:57:53]

Could you expand this comment with a sentence about whether there is a
guaranteed set of callbacks that will arrive? I guess it will have to be
best-effort, since we don't store the callbacks to re-delivery.

[mvendramini_hp, 2016/02/11 19:00:46]

I propose we either cross-reference the callbacks documentation at
components/dom_distiller/core/task_tracker.h somehow - or re-phrase it here.
Alternatively, I think the only guarantee that can be given here is that client
code is eligible to get callbacks as long as it holds onto a ViewerHandle.

Considering: 

1) task_tracker.h:45 "Called when the distilled article contents are available.
The DistilledArticleProto is owned by a TaskTracker instance and is invalidated
when the corresponding ViewerHandle is destroyed (or when the
DomDistillerService is destroyed)."

2) task_tracker.h:51 "Called when an article that is currently under
distillation is updated."

I'm not 100% confident on what kind of guarantees can be ascertained, given the
original task tracker documentation (see above).

Is it possible to get help from the original designer of task tracker for
documentation purposes? I reckon that would be cjhopman, I highly recommend his
input for a definitive answer on API guarantees in this case. @nyquist?

[nyquist, 2016/02/17 23:09:38]

So, the guarantee is:
- If this function returns nullptr, you will get no callbacks.
- If this function returns a ViewerHandle, you will at least get the
OnArticleFinished() callback. This includes two important cases:
  a) The article distillation is not finished yet, so the callback will trigger
at the same time as when the original distillation request gets it.
  b) The original distillation request has already gotten its callback, but the
ViewerHandle hasn't been deleted yet, so the TaskTracker is still around. When
that's the case, the callback will be posted immediately after adding the
viewer.
- If this function returns a ViewerHandle, and distillation has not finished
yet, you will also get all future callbacks for OnArticleUpdated().

[mvendramini_hp, 2016/02/18 12:27:02]

Done, expanded the doc-comment, please check it out.

+  virtual scoped_ptr<ViewerHandle> AddViewRequestDelegate(
+      ViewRequestDelegate* delegate,
+      const GURL& url) = 0;
+
   // Creates a default DistillerPage.
   virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPage(
       const gfx::Size& render_view_size) = 0;
   virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPageWithHandle(
       scoped_ptr<SourcePageHandle> handle) = 0;
 
   virtual void AddObserver(DomDistillerObserver* observer) = 0;
   virtual void RemoveObserver(DomDistillerObserver* observer) = 0;
 
   // Returns the DistilledPagePrefs owned by the instance of
@@ skipping 25 matching lines @@
   bool HasEntry(const std::string& entry_id) override;
   std::string GetUrlForEntry(const std::string& entry_id) override;
   std::vector<ArticleEntry> GetEntries() const override;
   scoped_ptr<ArticleEntry> RemoveEntry(const std::string& entry_id) override;
   scoped_ptr<ViewerHandle> ViewEntry(ViewRequestDelegate* delegate,
                                      scoped_ptr<DistillerPage> distiller_page,
                                      const std::string& entry_id) override;
   scoped_ptr<ViewerHandle> ViewUrl(ViewRequestDelegate* delegate,
                                    scoped_ptr<DistillerPage> distiller_page,
                                    const GURL& url) override;
+  scoped_ptr<ViewerHandle> AddViewRequestDelegate(ViewRequestDelegate* delegate,
+                                                  const GURL& url) override;
   scoped_ptr<DistillerPage> CreateDefaultDistillerPage(
       const gfx::Size& render_view_size) override;
   scoped_ptr<DistillerPage> CreateDefaultDistillerPageWithHandle(
       scoped_ptr<SourcePageHandle> handle) override;
   void AddObserver(DomDistillerObserver* observer) override;
   void RemoveObserver(DomDistillerObserver* observer) override;
   DistilledPagePrefs* GetDistilledPagePrefs() override;
 
  private:
   void CancelTask(TaskTracker* task);
@@ skipping 24 matching lines @@
 
   typedef ScopedVector<TaskTracker> TaskList;
   TaskList tasks_;
 
   DISALLOW_COPY_AND_ASSIGN(DomDistillerService);
 };
 
 }  // namespace dom_distiller
 
 #endif  // COMPONENTS_DOM_DISTILLER_CORE_DOM_DISTILLER_SERVICE_H_