[translate] Add translate ranker model loader.

components/translate/core/browser/translate_ranker.h

Index: components/translate/core/browser/translate_ranker.h
diff --git a/components/translate/core/browser/translate_ranker.h b/components/translate/core/browser/translate_ranker.h
index d8ed8ab3cc1db4e982bfbfe96e98e11631b2f54b..47b3ac80c43221c0fa9448600d572306245f438d 100644
--- a/components/translate/core/browser/translate_ranker.h
+++ b/components/translate/core/browser/translate_ranker.h
@@ -23,7 +23,10 @@ class TranslateEventProto;
 namespace translate {
 
 class TranslatePrefs;
-class TranslateURLFetcher;
+class TranslateRankerModelTraits;
+template <typename T>
+class ModelLoader;
+typedef ModelLoader<TranslateRankerModelTraits> TranslateRankerModelLoader;

[pasko, 2016/12/19 14:26:49]

is this repetition of the same typedef intentional?

[Roger McFarlane (Chromium), 2017/02/08 23:08:07]

There's no repetition here.

Forward declare TranslatePrefs
Forward declare TranslateRankerModelTraits;
Forward declare ModelLoader (template)
Forward declare TranslateRankerModelLoader (template specialization)

 
 // Features used to enable ranker query, enforcement and logging. Note that
 // enabling enforcement implies (forces) enabling queries.
@@ -57,9 +60,8 @@ class TranslateRanker {
   static std::unique_ptr<TranslateRanker> CreateForTesting(
       const std::string& model_data);
 
-  // Initiates downloading of the assist model data. This is a NOP if the model
-  // data has already been downloaded.
-  void FetchModelData();
+  // Starts the process to load the translate ranker model in the background.
+  void StartModelLoader();
 
   // Returns true if executing the ranker model in the translation prompt
   // context described by |translate_prefs|, |src_lang|, |dst_lang| and possibly
@@ -82,13 +84,18 @@ class TranslateRanker {
       std::vector<metrics::TranslateEventProto>* translate_events);
 
  private:
-  // The ID which is assigned to the underlying URLFetcher.
-  static constexpr int kFetcherId = 2;
+  typedef scoped_refptr<
+      base::RefCountedData<chrome_intelligence::TranslateRankerModel>>
+      SharedModelPtr;
+  typedef scoped_refptr<
+      const base::RefCountedData<chrome_intelligence::TranslateRankerModel>>
+      ConstSharedModelPtr;
 
   TranslateRanker();
 
   // Exposed for testing via FRIEND_TEST.
-  double CalculateScore(double accept_ratio,
+  double CalculateScore(const chrome_intelligence::TranslateRankerModel& model,
+                        double accept_ratio,
                         double decline_ratio,
                         double ignore_ratio,
                         const std::string& src_lang,
@@ -96,22 +103,21 @@ class TranslateRanker {
                         const std::string& app_locale,
                         const std::string& country);
 
-  // Called when the model download has completed.
-  void ParseModel(int id, bool success, const std::string& model_data);
+  // Called by the model loader when a new model is available.
+  void SetSharedModelPtr(
+      std::unique_ptr<chrome_intelligence::TranslateRankerModel> p);
 
-  // The translation ranker model.
-  std::unique_ptr<chrome_intelligence::TranslateRankerModel> model_;
+  // Returns a reference to a const shared model.
+  ConstSharedModelPtr GetSharedModelPtr() const;
 
-  // A URL fetcher to download translation ranker model data.
-  std::unique_ptr<TranslateURLFetcher> model_fetcher_;
+  // Protects copies/updates of the |shared_model_ptr_|.
+  mutable base::Lock lock_;

[pasko, 2016/12/19 14:26:49]

generally we discourage locks as a measure to avoid deadlocks, priority
inversion, etc. Please outline the threading model of the relevant classes in
the design doc. 

[gab, 2016/12/19 21:00:46]

+1, prefer non-thread-safe classes running on sequences (split logic in two or
more classes if there are distinct sequences of work sharing minimal state --
which they should share by posting tasks to each other instead of using a shared
copy unless there's a performance/memory cost to doing so).

Locks should only be used when doing all work in parallel (haven't verified
closely whether this is the case, will read about it in design doc update).

[Roger McFarlane (Chromium), 2017/02/08 23:08:07]

Done.

[Roger McFarlane (Chromium), 2017/02/08 23:08:08]

Done.

 
-  // The next time before which no new attempts to download the model should be
-  // attempted.
-  base::Time next_earliest_download_time_;
+  // A helper to load the model from disk cache or a URL.
+  std::unique_ptr<TranslateRankerModelLoader> model_loader_;
 
-  // Tracks the last time the translate ranker attempted to download its model.
-  // Used for UMA reporting of timing.
-  base::Time download_start_time_;
+  // The translation ranker model.
+  ConstSharedModelPtr shared_model_ptr_;
 
   // Saved cache of translate event protos.
   std::vector<metrics::TranslateEventProto> translate_events_cache_;