AndroidProvider: Add history for bookmarks if it doesn't exist.

chrome/browser/history/android/android_provider_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 "chrome/browser/history/android/android_provider_backend.h"
 
 #include "base/i18n/case_conversion.h"
 #include "chrome/browser/bookmarks/bookmark_service.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/browser/history/android/android_time.h"
@@ skipping 158 matching lines @@
   BroadcastNotifications(notifications);
   return true;
 }
 
 AndroidURLID AndroidProviderBackend::InsertHistoryAndBookmark(
     const HistoryAndBookmarkRow& values) {
   HistoryNotifications notifications;
 
   ScopedTransaction transaction(history_db_, thumbnail_db_);
 
-  AndroidURLID id = InsertHistoryAndBookmark(values, &notifications);
+  AndroidURLID id = InsertHistoryAndBookmark(values, &notifications, true);
   if (id) {
     transaction.Commit();
     BroadcastNotifications(notifications);
     return id;
   }
   return 0;
 }
 
 bool AndroidProviderBackend::DeleteHistoryAndBookmarks(
     const std::string& selection,
@@ skipping 160 matching lines @@
 
   if (!favicon->urls.empty())
     notifications->push_back(HistoryNotification(
         chrome::NOTIFICATION_FAVICON_CHANGED, favicon.release()));
 
   return true;
 }
 
 AndroidURLID AndroidProviderBackend::InsertHistoryAndBookmark(
     const HistoryAndBookmarkRow& values,
-    HistoryNotifications* notifications) {
+    HistoryNotifications* notifications,
+    bool update_tables) {
   if (!IsHistoryAndBookmarkRowValid(values))
     return false;
 
-  if (!EnsureInitializedAndUpdated())
+  if (update_tables && !EnsureInitializedAndUpdated())
     return 0;
 
   DCHECK(values.is_value_set_explicitly(HistoryAndBookmarkRow::URL));
   // Make a copy of values as we need change it during insert.
   HistoryAndBookmarkRow row = values;
   for (std::vector<SQLHandler*>::iterator i =
        sql_handlers_.begin(); i != sql_handlers_.end(); ++i) {
     if (!(*i)->Insert(&row))
       return 0;
   }
@@ skipping 89 matching lines @@
       bookmarks.push_back(row);
     }
   }
 
   // Don't delete the bookmark from bookmark model when deleting the history.
   if (!DeleteHistoryInternal(ids_set, false, notifications))
     return false;
 
   for (std::vector<HistoryAndBookmarkRow>::const_iterator i = bookmarks.begin();
        i != bookmarks.end(); ++i) {
-    if (!InsertHistoryAndBookmark(*i, notifications))
+    // Don't update the tables, otherwise, the bookmarks will be added to
+    // database during UpdateBookmark(), then the insertion will fail.
+    // We can't rely on UpdateBookmark() to insert the bookmarks into history
+    // database as the raw_url will be lost.
+    if (!InsertHistoryAndBookmark(*i, notifications, false))
       return false;
   }
-
   return true;
 }
 
 AndroidStatement* AndroidProviderBackend::QuerySearchTerms(
     const std::vector<SearchRow::ColumnID>& projections,
     const std::string& selection,
     const std::vector<string16>& selection_args,
     const std::string& sort_order) {
   if (projections.empty())
     return NULL;
@@ skipping 264 matching lines @@
   std::vector<BookmarkService::URLAndTitle> bookmarks;
   bookmark_service_->GetBookmarks(&bookmarks);
 
   if (bookmarks.empty())
     return true;
 
   std::vector<URLID> url_ids;
   for (std::vector<BookmarkService::URLAndTitle>::const_iterator i =
            bookmarks.begin(); i != bookmarks.end(); ++i) {
     URLID url_id = history_db_->GetRowForURL(i->url, NULL);
-    if (url_id == 0)
-      // TODO(michaelbai): Add a row to url and android_url table as the
-      // bookmark could be added manually by user or insertted by sync.
-      continue;
-
+    if (url_id == 0) {
+      URLRow url_row(i->url);
+      url_row.set_title(i->title);
+      url_row.set_last_visit(Time::UnixEpoch());

[sky, 2012/08/07 18:31:43]

You sure you don't want a time of now? And do you want to mark it hidden?

[michaelbai, 2012/08/07 19:21:32]

- Set the time to Time::UnixEpoch() consistent with what I did in line 461, the
Android CTS has test case for this. 

- What's the 'hidden' used for? I am not sure about making it hidden, as
HistoryBackend::DeleteAllHistory doesn't make the bookmark hidden, but
AddPageNoVisitForBookmark set it. Let me know if you think it should be hidden
here.


On 2012/08/07 18:31:43, sky wrote:

[sky, 2012/08/07 20:43:08]

Add a comment why then.
For one it's used by omnibox related code.

+      url_id = history_db_->AddURL(url_row);
+      if (url_id == 0) {
+        LOG(ERROR) << "Can not add url for the new bookmark";
+        return false;
+      }
+      if (!history_db_->AddAndroidURLRow(i->url.spec(), url_id))
+        return false;
+      if (!history_db_->AddBookmarkCacheRow(Time::UnixEpoch(),
+                            Time::UnixEpoch(), url_id))
+        return false;
+    }
     url_ids.push_back(url_id);
   }
 
   return history_db_->MarkURLsAsBookmarked(url_ids);
 }
 
 bool AndroidProviderBackend::UpdateFavicon() {
   ThumbnailDatabase::IconMappingEnumerator enumerator;
   if (!thumbnail_db_->InitIconMappingEnumerator(FAVICON, &enumerator))
     return false;
@@ skipping 370 matching lines @@
       return false;
 
     if (!history_db_->SetKeywordSearchTermsForURL(bookmark_row.url_id(),
                           values.template_url_id(), values.search_term()))
       return false;
   }
   return true;
 }
 
 }  // namespace history