Seculative resource prefetching for URLs CL.

chrome/browser/predictors/resource_prefetch_predictor_tables.h

+// Copyright (c) 2012 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 CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_TABLES_H_
+#define CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_TABLES_H_
+#pragma once
+
+#include "chrome/browser/predictors/predictor_table_base.h"
+
+#include <string>
+#include <vector>
+
+#include "googleurl/src/gurl.h"
+#include "webkit/glue/resource_type.h"
+
+namespace predictors {
+
+// Interface for database tables used by the ResourcePrefetchPredictor.
+// All methods except the constructor and destructor need to be called on the DB
+// thread.
+// NOTE: This class is named in plural as it will hold other tables shortly.
+//
+// TODO(shishir): Move to composition model instead of implemented inheritance.
+class ResourcePrefetchPredictorTables : public PredictorTableBase {
+ public:
+  struct UrlTableRow {
+    UrlTableRow();
+    UrlTableRow(const UrlTableRow& other);
+    UrlTableRow(const std::string& main_frame_url,
+                const std::string& resource_url,
+                ResourceType::Type resource_type,
+                int number_of_hits,
+                int number_of_misses,
+                int consecutive_misses,
+                double average_position);
+    void UpdateScore();
+    bool operator==(const UrlTableRow& rhs) const;
+
+    GURL main_frame_url;
+    GURL resource_url;
+    ResourceType::Type resource_type;
+    int number_of_hits;
+    int number_of_misses;
+    int consecutive_misses;
+    double average_position;
+
+    // Not stored.
+    float score;
+  };
+  typedef std::vector<UrlTableRow> UrlTableRows;
+
+  // Sorts the UrlTableRows by score, descending.
+  struct UrlTableRowSorter {
+    bool operator()(const UrlTableRow& x, const UrlTableRow& y) const;
+  };
+
+  // |url_row_buffer| should be empty for GetAllRows.
+  virtual void GetAllRows(UrlTableRows* url_row_buffer);
+  virtual void UpdateRowsForUrl(const GURL& main_page_url,
+                                const UrlTableRows& row_buffer);
+  virtual void DeleteRowsForUrls(const std::vector<GURL>& urls);
+  virtual void DeleteAllRows();
+
+ private:
+  friend class PredictorDatabaseInternal;

[willchan no longer on Chromium, 2012/06/18 20:15:12]

I'm surprised at this use of a friend declaration. Can you explain why this is a
clean design?

[Shishir, 2012/06/18 20:38:33]

This is to give the PredictorDatabaseInternal access to constructor and
initialization functions that the rest of the callers should not call.

[willchan no longer on Chromium, 2012/06/19 00:38:26]

Why the constructor? What harm will happen from someone calling the constructor?

LogDatabaseStats() is public in PredictorTableBase. What's the point of hiding
it?

I see...the primary consumer is the FooPredictor classes you have. This
friendship pattern you are relying on is creating a tight coupling between
PredictorDatabase and all its database tables. If that's what you want, it's
fine. The friendship is definitely allowing you to establish an interface
boundary between FooPredictor and the FooPredictorTable.

What would be cleaner, albeit more code, is to get rid of this tight coupling
and replace it with a FooPredictorTable interface and a FooPredictorTableImpl
that did the initialization and what not (composing the PredictorTableBase
rather than using implementation inheritance). But the FooPredictor would only
interact with the FooPredictorTable interface, not the FooPredictorTableImpl, so
you could do dependency injection and what not. Also, it's unfortunate that
FooPredictor has to be aware that FooPredictorTable is implemented via the DB
thread. Really, the use of the DB thread should be a detail internal to the
FooPredictorTableImpl, and the interface should take a callback. The real impl
would do a PostTaskAndReply() to the DB thread, but the client wouldn't have to
do this. And then for a test implementation, you could inject a mock that didn't
do the PostTaskAndReply.

I recommend these steps for cleaner design, but will not require them since this
isn't my code to maintain. I defer to y'all.

+  friend class MockResourcePrefetchPredictorTables;
+
+  ResourcePrefetchPredictorTables();
+  virtual ~ResourcePrefetchPredictorTables();
+
+  // PredictorTableBase methods.
+  virtual void CreateTableIfNonExistent() OVERRIDE;
+  virtual void LogDatabaseStats() OVERRIDE;
+
+  DISALLOW_COPY_AND_ASSIGN(ResourcePrefetchPredictorTables);
+};
+
+}  // namespace predictors
+
+#endif  // CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_TABLES_H_