[Thumbnails DB] Allow setting last_requested time when accessing favicons.

components/history/core/browser/history_backend.cc

 // 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.
 
 #include "components/history/core/browser/history_backend.h"
 
 #include <algorithm>
 #include <functional>
 #include <list>
 #include <map>
@@ skipping 1491 matching lines @@
   favicon_base::IconType icon_type;
   if (!thumbnail_db_->GetFaviconHeader(largest_icon.icon_id, &icon_url,
                                        &icon_type)) {
     return;
   }
 
   base::Time last_updated;
   favicon_base::FaviconRawBitmapResult bitmap_result;
   bitmap_result.icon_url = icon_url;
   bitmap_result.icon_type = icon_type;
-  if (!thumbnail_db_->GetFaviconBitmap(largest_icon.bitmap_id,
-                                       &last_updated, nullptr,
-                                       &bitmap_result.bitmap_data,
-                                       &bitmap_result.pixel_size)) {
+  if (!thumbnail_db_->GetFaviconBitmap(
+          largest_icon.bitmap_id, /*access_time=*/Time::Now(), &last_updated,

[jkrcal, 2017/05/02 13:57:25]

For all call 3 sites here, I need to pass Time() in incognito profiles, instead.
What is the easiest way to find it out? Should I pass a |is_incognito| bit into
the backend upon construction?

[jkrcal, 2017/05/15 14:26:57]

Because of the privacy discussion, I still believe I need to implement this. 
What is the easiest way to find out here that the user is in incognito?

[sky, 2017/05/15 15:57:13]

You will have to change callers to supply whether in incognito mode or not.

[jkrcal, 2017/05/15 18:36:29]

Huh, now I see better why you pushed in the direction of being okay privacy-wise
:| Do I get it right that the history service/backend/databases are shared
between the profile and its incognito variant (so that I cannot pass the
incognito bit into the ctor)? 

Is it then always the callers of HistoryService who make sure incognito is
respected?

Adding the incognito bit into whole GetFavicon* API feels really ugly. I am
considering a different approach:
 - change the factory of FaviconService to create separate instances for the
main profile and for incognito. FaviconService is quite a thin layer on top of
HistoryBackend so it feels okay to me, memory-wise.
 - let the last_requested time be updated only from calls from non-icognito
FaviconService.

WDYT?

+          &bitmap_result.bitmap_data, &bitmap_result.pixel_size)) {
     return;
   }
 
   bitmap_result.expired =
       (Time::Now() - last_updated) > TimeDelta::FromDays(kFaviconRefetchDays);
   if (bitmap_result.is_valid())
     *favicon_bitmap_result = bitmap_result;
 
   LOCAL_HISTOGRAM_TIMES("History.GetLargestFaviconForURL",
                         TimeTicks::Now() - beginning_time);
@@ skipping 461 matching lines @@
   return favicon_bitmaps_changed;
 }
 
 bool HistoryBackend::IsFaviconBitmapDataEqual(
     FaviconBitmapID bitmap_id,
     const scoped_refptr<base::RefCountedMemory>& new_bitmap_data) {
   if (!new_bitmap_data)
     return false;
 
   scoped_refptr<base::RefCountedMemory> original_bitmap_data;
-  thumbnail_db_->GetFaviconBitmap(bitmap_id, nullptr, nullptr,
-                                  &original_bitmap_data, nullptr);
+  thumbnail_db_->GetFaviconBitmap(bitmap_id, /*access_time=*/Time::Now(),
+                                  nullptr, &original_bitmap_data, nullptr);
   return new_bitmap_data->Equals(original_bitmap_data);
 }
 
 bool HistoryBackend::GetFaviconsFromDB(
     const GURL& page_url,
     int icon_types,
     const std::vector<int>& desired_sizes,
     std::vector<favicon_base::FaviconRawBitmapResult>* favicon_bitmap_results) {
   DCHECK(favicon_bitmap_results);
   favicon_bitmap_results->clear();
@@ skipping 67 matching lines @@
   if (!thumbnail_db_->GetFaviconHeader(best_favicon_id, &icon_url,
                                        &icon_type)) {
     return false;
   }
 
   for (size_t i = 0; i < best_bitmap_ids.size(); ++i) {
     base::Time last_updated;
     favicon_base::FaviconRawBitmapResult bitmap_result;
     bitmap_result.icon_url = icon_url;
     bitmap_result.icon_type = icon_type;
-    if (!thumbnail_db_->GetFaviconBitmap(best_bitmap_ids[i], &last_updated,
-                                         nullptr, &bitmap_result.bitmap_data,
-                                         &bitmap_result.pixel_size)) {
+    if (!thumbnail_db_->GetFaviconBitmap(
+            best_bitmap_ids[i], /*access_time=*/Time::Now(), &last_updated,
+            &bitmap_result.bitmap_data, &bitmap_result.pixel_size)) {
       return false;
     }
 
     bitmap_result.expired =
         (Time::Now() - last_updated) > TimeDelta::FromDays(kFaviconRefetchDays);
     if (bitmap_result.is_valid())
       favicon_bitmap_results->push_back(bitmap_result);
   }
   return true;
 }
@@ skipping 562 matching lines @@
   // transaction is currently open.
   db_->CommitTransaction();
   db_->Vacuum();
   db_->BeginTransaction();
   db_->GetStartDate(&first_recorded_time_);
 
   return true;
 }
 
 }  // namespace history