Commit 144fb7b8 authored by ssid's avatar ssid Committed by Commit Bot

Reduce the memory usage of bookmarks storage

This CL does:
1. Url index uses flat_set instead of set.
2. Do not load image data and page data from bookmarks metadata into
   memory these are deprecated.
3. BookmarkNode stores ptr to icon_url instead of GURL

BUG=691698

Review-Url: https://codereview.chromium.org/2883523002
Cr-Commit-Position: refs/heads/master@{#476158}
parent 6dfadefe
......@@ -235,7 +235,8 @@ FaviconData GetFaviconData(BookmarkModel* model,
model->GetFavicon(node);
observer.WaitForGetFavicon();
}
return FaviconData(model->GetFavicon(node), node->icon_url());
return FaviconData(model->GetFavicon(node),
node->icon_url() ? *node->icon_url() : GURL());
}
// Sets the favicon for |profile| and |node|. |profile| may be
......
......@@ -433,6 +433,11 @@ void BookmarkCodec::DecodeMetaInfoHelper(
const std::string& prefix,
BookmarkNode::MetaInfoMap* meta_info_map) {
for (base::DictionaryValue::Iterator it(dict); !it.IsAtEnd(); it.Advance()) {
// Deprecated keys should be excluded after removing enhanced bookmarks
// feature crrev.com/1638413003.
if (base::StartsWith(it.key(), "stars.", base::CompareCase::SENSITIVE))
continue;
if (it.value().IsType(base::Value::Type::DICTIONARY)) {
const base::DictionaryValue* subdict;
it.value().GetAsDictionary(&subdict);
......
......@@ -482,7 +482,7 @@ void BookmarkModel::OnFaviconsChanged(const std::set<GURL>& page_urls,
base::AutoLock url_lock(url_lock_);
for (const BookmarkNode* node : nodes_ordered_by_url_set_) {
if (icon_url == node->icon_url())
if (node->icon_url() && icon_url == *node->icon_url())
to_update.insert(node);
}
}
......
......@@ -123,7 +123,7 @@ void BookmarkNode::Initialize(int64_t id) {
}
void BookmarkNode::InvalidateFavicon() {
icon_url_ = GURL();
icon_url_.reset();
favicon_ = gfx::Image();
favicon_type_ = favicon_base::INVALID_ICON;
favicon_state_ = INVALID_FAVICON;
......
......@@ -10,6 +10,7 @@
#include <memory>
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/task/cancelable_task_tracker.h"
#include "base/time/time.h"
#include "components/bookmarks/browser/titled_url_node.h"
......@@ -69,7 +70,7 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode>, public TitledUrlNode {
// Returns the favicon's URL. Returns an empty URL if there is no favicon
// associated with this bookmark.
const GURL& icon_url() const { return icon_url_; }
const GURL* icon_url() const { return icon_url_ ? icon_url_.get() : nullptr; }
Type type() const { return type_; }
void set_type(Type type) { type_ = type; }
......@@ -134,7 +135,7 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode>, public TitledUrlNode {
// Sets the favicon's URL.
void set_icon_url(const GURL& icon_url) {
icon_url_ = icon_url;
icon_url_ = base::MakeUnique<GURL>(icon_url);
}
// Returns the favicon. In nearly all cases you should use the method
......@@ -179,7 +180,7 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode>, public TitledUrlNode {
favicon_base::IconType favicon_type_;
// The URL of the node's favicon.
GURL icon_url_;
std::unique_ptr<GURL> icon_url_;
// The loading state of the favicon.
FaviconState favicon_state_;
......
......@@ -207,16 +207,10 @@ bool TitledUrlIndex::GetResultsMatchingTerm(
while (i != index_.end() &&
i->first.size() >= term.size() &&
term.compare(0, term.size(), i->first, 0, term.size()) == 0) {
#if !defined(OS_ANDROID)
prefix_matches->insert(i->second.begin(), i->second.end());
#else
// Work around a bug in the implementation of std::set::insert in the STL
// used on android (http://crbug.com/367050).
for (TitledUrlNodeSet::const_iterator n = i->second.begin();
n != i->second.end();
++n)
n != i->second.end(); ++n) {
prefix_matches->insert(prefix_matches->end(), *n);
#endif
}
++i;
}
if (!first_term) {
......
......@@ -8,10 +8,10 @@
#include <stddef.h>
#include <map>
#include <set>
#include <string>
#include <vector>
#include "base/containers/flat_set.h"
#include "base/macros.h"
#include "base/strings/string16.h"
#include "components/query_parser/query_parser.h"
......@@ -50,7 +50,7 @@ class TitledUrlIndex {
private:
using TitledUrlNodes = std::vector<const TitledUrlNode*>;
using TitledUrlNodeSet = std::set<const TitledUrlNode*>;
using TitledUrlNodeSet = base::flat_set<const TitledUrlNode*>;
using Index = std::map<base::string16, TitledUrlNodeSet>;
// Constructs |sorted_nodes| by copying the matches in |matches| and sorting
......
......@@ -5,9 +5,10 @@
#ifndef COMPONENTS_BOOKMARKS_BROWSER_TITLED_URL_NODE_SORTER_H_
#define COMPONENTS_BOOKMARKS_BROWSER_TITLED_URL_NODE_SORTER_H_
#include <set>
#include <vector>
#include "base/containers/flat_set.h"
namespace bookmarks {
class TitledUrlNode;
......@@ -15,7 +16,7 @@ class TitledUrlNode;
class TitledUrlNodeSorter {
public:
using TitledUrlNodes = std::vector<const TitledUrlNode*>;
using TitledUrlNodeSet = std::set<const TitledUrlNode*>;
using TitledUrlNodeSet = base::flat_set<const TitledUrlNode*>;
virtual ~TitledUrlNodeSorter() {};
......
......@@ -968,7 +968,9 @@ void BookmarkChangeProcessor::SetSyncNodeFavicon(
sync_node->GetBookmarkSpecifics());
updated_specifics.set_favicon(favicon_bytes->front(),
favicon_bytes->size());
updated_specifics.set_icon_url(bookmark_node->icon_url().spec());
updated_specifics.set_icon_url(bookmark_node->icon_url()
? bookmark_node->icon_url()->spec()
: std::string());
sync_node->SetBookmarkSpecifics(updated_specifics);
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment