Disable tracking of visits to bookmarks in incognito

chrome/browser/ntp_snippets/bookmark_last_visit_updater.cc

 // Copyright 2016 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 "chrome/browser/ntp_snippets/bookmark_last_visit_updater.h"
 
 #include "chrome/common/features.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/ntp_snippets/bookmarks/bookmark_last_visit_utils.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_handle.h"
 
 namespace {
 
 bool IsMobilePlatform() {
 #if BUILDFLAG(ANDROID_JAVA_UI)  // There are no tab helpers on iOS.
   return true;
 #else
   return false;
 #endif
 }
 
 }  // namespace
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(BookmarkLastVisitUpdater);
 
 BookmarkLastVisitUpdater::~BookmarkLastVisitUpdater() {
   // In unit-tests on desktop, the bookmark_model is null.
-  if (!bookmark_model_) {
+  if (!bookmark_model_ || !tracking_enabled_) {
     return;

[Bernhard Bauer, 2016/12/12 15:06:37]

FWIW, using a base::ScopedObserver would let us avoid this check, but maybe we
can avoid creating this object completely (see below).

[jkrcal, 2016/12/14 13:40:44]

Done.

   }
   bookmark_model_->RemoveObserver(this);
 }
 
 // static
 void BookmarkLastVisitUpdater::CreateForWebContentsWithBookmarkModel(
     content::WebContents* web_contents,
     bookmarks::BookmarkModel* bookmark_model) {
   web_contents->SetUserData(UserDataKey(), new BookmarkLastVisitUpdater(
                                                web_contents, bookmark_model));
 }
 
 BookmarkLastVisitUpdater::BookmarkLastVisitUpdater(
     content::WebContents* web_contents,
     bookmarks::BookmarkModel* bookmark_model)
     : content::WebContentsObserver(web_contents),
       bookmark_model_(bookmark_model),
       web_contents_(web_contents) {
+  // Switch off tracking visits for off the record profiles.

[Bernhard Bauer, 2016/12/12 15:06:37]

Nit: the code here uses the term "off the record" because no one has renamed the
method, but outside of the most commonly used term is "incognito", so I think we
should use that as much as possible.

[jkrcal, 2016/12/14 13:40:44]

Done.

+  content::BrowserContext* browser_context = web_contents->GetBrowserContext();
+  tracking_enabled_ = !browser_context->IsOffTheRecord();
+
   // In unit-tests on desktop, the bookmark_model is null.
-  if (!bookmark_model_) {
+  if (!bookmark_model_ || !tracking_enabled_) {

[Bernhard Bauer, 2016/12/12 15:06:38]

So, what does this class actually do if tracking is disabled (or we don't have a
bookmark model)? Could we just skip creating it completely?

[jkrcal, 2016/12/14 13:40:44]

Done.

     return;
   }
   bookmark_model->AddObserver(this);
 }
 
 void BookmarkLastVisitUpdater::DidStartNavigation(
     content::NavigationHandle* navigation_handle) {
   // Mark visited bookmark when the navigation starts (may end somewhere else
   // due to server-side redirects).
   NewURLVisited(navigation_handle);
 }
 
 void BookmarkLastVisitUpdater::DidRedirectNavigation(
     content::NavigationHandle* navigation_handle) {
   // Mark visited bookmark also after each redirect.
   NewURLVisited(navigation_handle);
 }
 
 void BookmarkLastVisitUpdater::NewURLVisited(
     content::NavigationHandle* navigation_handle) {
   // In unit-tests on desktop, the bookmark_model is null.
   if (!navigation_handle->IsInMainFrame() || navigation_handle->IsErrorPage() ||
-      !bookmark_model_) {
+      !bookmark_model_ || !tracking_enabled_) {
     return;
   }
 
   ntp_snippets::UpdateBookmarkOnURLVisitedInMainFrame(
       bookmark_model_, navigation_handle->GetURL(), IsMobilePlatform());
 }
 
 void BookmarkLastVisitUpdater::BookmarkNodeAdded(
     bookmarks::BookmarkModel* model,
     const bookmarks::BookmarkNode* parent,
     int index) {
   const GURL& new_bookmark_url = parent->GetChild(index)->url();
 
   if (new_bookmark_url == web_contents_->GetLastCommittedURL()) {
     // Consider in this TabHelper only bookmarks created from this tab (and not
     // the ones created from other tabs or created through bookmark sync).
     ntp_snippets::UpdateBookmarkOnURLVisitedInMainFrame(
         bookmark_model_, new_bookmark_url, IsMobilePlatform());
   }
 }