Improve syncing between dom distiller and the print preview request

components/dom_distiller/core/dom_distiller_service.h

Index: components/dom_distiller/core/dom_distiller_service.h
diff --git a/components/dom_distiller/core/dom_distiller_service.h b/components/dom_distiller/core/dom_distiller_service.h
index 5d2558d1142b68467f6434f8b538663a16cd7762..1b4c627a54729c9bc3da563352d960b13ebb1652 100644
--- a/components/dom_distiller/core/dom_distiller_service.h
+++ b/components/dom_distiller/core/dom_distiller_service.h
@@ -92,6 +92,18 @@ class DomDistillerServiceInterface {
       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;
@@ -137,6 +149,8 @@ class DomDistillerService : public DomDistillerServiceInterface {
   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(