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

components/history/core/browser/history_backend.cc

Index: components/history/core/browser/history_backend.cc
diff --git a/components/history/core/browser/history_backend.cc b/components/history/core/browser/history_backend.cc
index 2ab8db29706c31618fd98fa350acc6caad45a35c..9152bfb0e11a5717a95b7b551f002d1306f590aa 100644
--- a/components/history/core/browser/history_backend.cc
+++ b/components/history/core/browser/history_backend.cc
@@ -1509,10 +1509,9 @@ void HistoryBackend::GetLargestFaviconForURL(
   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;
   }
 
@@ -1994,8 +1993,8 @@ bool HistoryBackend::IsFaviconBitmapDataEqual(
     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);
 }
 
@@ -2083,9 +2082,9 @@ bool HistoryBackend::GetFaviconBitmapResultsForBestMatch(
     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;
     }