Move common distiller code to superclass

components/dom_distiller/core/dom_distiller_request_view_base.h

Index: components/dom_distiller/core/dom_distiller_request_view_base.h
diff --git a/components/dom_distiller/core/dom_distiller_request_view_base.h b/components/dom_distiller/core/dom_distiller_request_view_base.h
new file mode 100644
index 0000000000000000000000000000000000000000..ca5c757358d9a8b921bb3119805c462805fe9f12
--- /dev/null
+++ b/components/dom_distiller/core/dom_distiller_request_view_base.h
@@ -0,0 +1,79 @@
+// 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_DOM_DISTILLER_CORE_DOM_DISTILLER_REQUEST_VIEW_BASE_H_
+#define COMPONENTS_DOM_DISTILLER_CORE_DOM_DISTILLER_REQUEST_VIEW_BASE_H_
+
+#include <sstream>
+#include <string>
+#include <vector>
+
+#include "base/memory/ref_counted_memory.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/strings/utf_string_conversions.h"
+#include "components/dom_distiller/core/distilled_page_prefs.h"
+#include "components/dom_distiller/core/dom_distiller_service.h"
+#include "components/dom_distiller/core/task_tracker.h"
+#include "components/dom_distiller/core/viewer.h"
+#include "content/public/browser/url_data_source.h"
+#include "net/base/url_util.h"
+
+namespace dom_distiller {
+
+// This interface is used to abstract the data callback from the distiller. The
+// callbacks for different platforms have different numbers of parameters
+// (namely IOS and android) which makes this necessary.

[nyquist, 2015/04/24 22:13:09]

Nit: iOS and Android

[mdjones, 2015/04/25 00:55:03]

Done.

+class DistillerDataCallback {
+ public:
+  virtual ~DistillerDataCallback(){};
+  virtual void runCallback(std::string& data) = 0;

[nyquist, 2015/04/24 22:13:09]

RunCallback. Also, could the param be const?

[mdjones, 2015/04/25 00:55:03]

Fixed function name; I could make this const but it would require me making a
copy of the string to make a RefCountedString.

This whole callback class concept may not actually be around too long once early
template loading lands. That requires some work on IOS though.

+};
+
+// Handles receiving data asynchronously for a specific entry, and passing
+// it along to the data callback for the data source. Lifetime matches that of
+// the current main frame's page in the Viewer instance.
+class DomDistillerRequestViewBase : public ViewRequestDelegate {
+ public:
+  explicit DomDistillerRequestViewBase(
+      scoped_ptr<DistillerDataCallback> callback,
+      DistilledPagePrefs* distilled_page_prefs);
+  ~DomDistillerRequestViewBase() override;
+
+  // Flag this request as an error and send the error page template.
+  void flagAsErrorPage();

[nyquist, 2015/04/24 22:13:09]

FlagAsErrorPage

[mdjones, 2015/04/25 00:55:03]

Done.

+
+  // ViewRequestDelegate implementation:
+  void OnArticleReady(const DistilledArticleProto* article_proto) override;
+
+  void OnArticleUpdated(ArticleDistillationUpdate article_update) override;
+
+  void TakeViewerHandle(scoped_ptr<ViewerHandle> viewer_handle);
+
+ protected:
+  // Sends JavaScript to the attached Viewer, buffering data if the viewer isn't
+  // ready.

[nyquist, 2015/04/24 22:13:09]

Also, maybe add the sanity check comment of: "When the viewer becomes ready, the
buffered data should be flushed" or something along those lines?

I wonder though, is that something this base class could help out with possibly?
Maybe having some virtual method "IsReady()" which would be called, and later
make the subclasses call "OnReady()", which would flush the buffer? I'm not sure
it's worth it though. Might just be confusing. Any thoughts?

Secondly, maybe mention here, that the buffers are self contained (and
terminating JavaScript code), so it should be safe to append to an existing
buffer?

[mdjones, 2015/04/25 00:55:03]

There are a few things that need to be worked out on IOS before this makes
sense. For example, I would need the WebContents in the IOS version and I am
unsure of how difficult that is.

[nyquist, 2015/04/25 03:15:55]

Yeah. iOS doesn't have WebContents. I was thinking about having one method for
sending the JS in the subclass liked now, but having this base class buffer the
JS. However the subclass's would then have to tell this parent class to flush
the buffer once the viewer was ready. And this base class would have to be able
to call a method in the subclass to see whether the viewer was ready to receive
JS or not.

Anyway, no point of doing this in this refactoring CL.

+  virtual void SendJavaScript(const std::string& buffer) = 0;
+
+  // The handle to the view request towards the DomDistillerService. It
+  // needs to be kept around to ensure the distillation request finishes.
+  scoped_ptr<ViewerHandle> viewer_handle_;
+
+  // Holds the callback to where the data retrieved is sent back.
+  // content::URLDataSource::GotDataCallback callback_;

[nyquist, 2015/04/24 22:13:09]

Remove?

[mdjones, 2015/04/25 00:55:03]

Done.

+  scoped_ptr<DistillerDataCallback> callback_;
+
+  // Number of pages of the distilled article content that have been rendered by
+  // the viewer.
+  int page_count_;
+
+  // Interface for accessing preferences for distilled pages.
+  DistilledPagePrefs* distilled_page_prefs_;
+
+  // Flag to tell this observer that the web contents are in an error state.
+  bool is_error_page_;

[nyquist, 2015/04/24 22:13:09]

Is this ever read anywhere?

[mdjones, 2015/04/25 00:55:03]

Made a proper getter (IsErrorPage) for this which is read in viewer source.

+};
+
+}  // namespace dom_distiller
+
+#endif  // COMPONENTS_DOM_DISTILLER_CORE_DOM_DISTILLER_REQUEST_VIEW_BASE_H_