Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(366)

Side by Side Diff: components/bookmarks/browser/bookmark_storage.cc

Issue 373153002: Fixed use-after-free in LoadCallback in bookmark_storage.cc (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « components/bookmarks/browser/bookmark_storage.h ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/bookmarks/browser/bookmark_storage.h" 5 #include "components/bookmarks/browser/bookmark_storage.h"
6 6
7 #include "base/bind.h" 7 #include "base/bind.h"
8 #include "base/compiler_specific.h" 8 #include "base/compiler_specific.h"
9 #include "base/file_util.h" 9 #include "base/file_util.h"
10 #include "base/json/json_file_value_serializer.h" 10 #include "base/json/json_file_value_serializer.h"
(...skipping 31 matching lines...) Expand 10 before | Expand all | Expand 10 after
42 if (node->url().is_valid()) 42 if (node->url().is_valid())
43 details->index()->Add(node); 43 details->index()->Add(node);
44 } else { 44 } else {
45 for (int i = 0; i < node->child_count(); ++i) 45 for (int i = 0; i < node->child_count(); ++i)
46 AddBookmarksToIndex(details, node->GetChild(i)); 46 AddBookmarksToIndex(details, node->GetChild(i));
47 } 47 }
48 } 48 }
49 49
50 void LoadCallback(const base::FilePath& path, 50 void LoadCallback(const base::FilePath& path,
51 const base::WeakPtr<BookmarkStorage>& storage, 51 const base::WeakPtr<BookmarkStorage>& storage,
52 BookmarkLoadDetails* details, 52 scoped_ptr<BookmarkLoadDetails> details,
53 base::SequencedTaskRunner* task_runner) { 53 base::SequencedTaskRunner* task_runner) {
54 startup_metric_utils::ScopedSlowStartupUMA 54 startup_metric_utils::ScopedSlowStartupUMA
55 scoped_timer("Startup.SlowStartupBookmarksLoad"); 55 scoped_timer("Startup.SlowStartupBookmarksLoad");
56 bool load_index = false; 56 bool load_index = false;
57 bool bookmark_file_exists = base::PathExists(path); 57 bool bookmark_file_exists = base::PathExists(path);
58 if (bookmark_file_exists) { 58 if (bookmark_file_exists) {
59 JSONFileValueSerializer serializer(path); 59 JSONFileValueSerializer serializer(path);
60 scoped_ptr<base::Value> root(serializer.Deserialize(NULL, NULL)); 60 scoped_ptr<base::Value> root(serializer.Deserialize(NULL, NULL));
61 61
62 if (root.get()) { 62 if (root.get()) {
(...skipping 26 matching lines...) Expand all
89 const BookmarkPermanentNodeList& extra_nodes = details->extra_nodes(); 89 const BookmarkPermanentNodeList& extra_nodes = details->extra_nodes();
90 for (size_t i = 0; i < extra_nodes.size(); ++i) { 90 for (size_t i = 0; i < extra_nodes.size(); ++i) {
91 if (!extra_nodes[i]->empty()) { 91 if (!extra_nodes[i]->empty()) {
92 load_index = true; 92 load_index = true;
93 break; 93 break;
94 } 94 }
95 } 95 }
96 96
97 if (load_index) { 97 if (load_index) {
98 TimeTicks start_time = TimeTicks::Now(); 98 TimeTicks start_time = TimeTicks::Now();
99 AddBookmarksToIndex(details, details->bb_node()); 99 AddBookmarksToIndex(details.get(), details->bb_node());
100 AddBookmarksToIndex(details, details->other_folder_node()); 100 AddBookmarksToIndex(details.get(), details->other_folder_node());
101 AddBookmarksToIndex(details, details->mobile_folder_node()); 101 AddBookmarksToIndex(details.get(), details->mobile_folder_node());
102 for (size_t i = 0; i < extra_nodes.size(); ++i) 102 for (size_t i = 0; i < extra_nodes.size(); ++i)
103 AddBookmarksToIndex(details, extra_nodes[i]); 103 AddBookmarksToIndex(details.get(), extra_nodes[i]);
104 UMA_HISTOGRAM_TIMES("Bookmarks.CreateBookmarkIndexTime", 104 UMA_HISTOGRAM_TIMES("Bookmarks.CreateBookmarkIndexTime",
105 TimeTicks::Now() - start_time); 105 TimeTicks::Now() - start_time);
106 } 106 }
107 107
108 task_runner->PostTask(FROM_HERE, 108 task_runner->PostTask(FROM_HERE,
109 base::Bind(&BookmarkStorage::OnLoadFinished, storage)); 109 base::Bind(&BookmarkStorage::OnLoadFinished, storage,
110 base::Passed(&details)));
110 } 111 }
111 112
112 } // namespace 113 } // namespace
113 114
114 // BookmarkLoadDetails --------------------------------------------------------- 115 // BookmarkLoadDetails ---------------------------------------------------------
115 116
116 BookmarkLoadDetails::BookmarkLoadDetails( 117 BookmarkLoadDetails::BookmarkLoadDetails(
117 BookmarkPermanentNode* bb_node, 118 BookmarkPermanentNode* bb_node,
118 BookmarkPermanentNode* other_folder_node, 119 BookmarkPermanentNode* other_folder_node,
119 BookmarkPermanentNode* mobile_folder_node, 120 BookmarkPermanentNode* mobile_folder_node,
(...skipping 35 matching lines...) Expand 10 before | Expand all | Expand 10 after
155 } 156 }
156 157
157 BookmarkStorage::~BookmarkStorage() { 158 BookmarkStorage::~BookmarkStorage() {
158 if (writer_.HasPendingWrite()) 159 if (writer_.HasPendingWrite())
159 writer_.DoScheduledWrite(); 160 writer_.DoScheduledWrite();
160 } 161 }
161 162
162 void BookmarkStorage::LoadBookmarks( 163 void BookmarkStorage::LoadBookmarks(
163 scoped_ptr<BookmarkLoadDetails> details, 164 scoped_ptr<BookmarkLoadDetails> details,
164 const scoped_refptr<base::SequencedTaskRunner>& task_runner) { 165 const scoped_refptr<base::SequencedTaskRunner>& task_runner) {
165 DCHECK(!details_.get());
166 DCHECK(details);
167 details_ = details.Pass();
168 sequenced_task_runner_->PostTask(FROM_HERE, 166 sequenced_task_runner_->PostTask(FROM_HERE,
169 base::Bind(&LoadCallback, 167 base::Bind(&LoadCallback,
170 writer_.path(), 168 writer_.path(),
171 weak_factory_.GetWeakPtr(), 169 weak_factory_.GetWeakPtr(),
172 details_.get(), 170 base::Passed(&details),
173 task_runner)); 171 task_runner));
174 } 172 }
175 173
176 void BookmarkStorage::ScheduleSave() { 174 void BookmarkStorage::ScheduleSave() {
177 writer_.ScheduleWrite(this); 175 writer_.ScheduleWrite(this);
178 } 176 }
179 177
180 void BookmarkStorage::BookmarkModelDeleted() { 178 void BookmarkStorage::BookmarkModelDeleted() {
181 // We need to save now as otherwise by the time SaveNow is invoked 179 // We need to save now as otherwise by the time SaveNow is invoked
182 // the model is gone. 180 // the model is gone.
183 if (writer_.HasPendingWrite()) 181 if (writer_.HasPendingWrite())
184 SaveNow(); 182 SaveNow();
185 model_ = NULL; 183 model_ = NULL;
186 } 184 }
187 185
188 bool BookmarkStorage::SerializeData(std::string* output) { 186 bool BookmarkStorage::SerializeData(std::string* output) {
189 BookmarkCodec codec; 187 BookmarkCodec codec;
190 scoped_ptr<base::Value> value(codec.Encode(model_)); 188 scoped_ptr<base::Value> value(codec.Encode(model_));
191 JSONStringValueSerializer serializer(output); 189 JSONStringValueSerializer serializer(output);
192 serializer.set_pretty_print(true); 190 serializer.set_pretty_print(true);
193 return serializer.Serialize(*(value.get())); 191 return serializer.Serialize(*(value.get()));
194 } 192 }
195 193
196 void BookmarkStorage::OnLoadFinished() { 194 void BookmarkStorage::OnLoadFinished(scoped_ptr<BookmarkLoadDetails> details) {
197 if (!model_) 195 if (!model_)
198 return; 196 return;
199 197
200 model_->DoneLoading(details_.Pass()); 198 model_->DoneLoading(details.Pass());
201 } 199 }
202 200
203 bool BookmarkStorage::SaveNow() { 201 bool BookmarkStorage::SaveNow() {
204 if (!model_ || !model_->loaded()) { 202 if (!model_ || !model_->loaded()) {
205 // We should only get here if we have a valid model and it's finished 203 // We should only get here if we have a valid model and it's finished
206 // loading. 204 // loading.
207 NOTREACHED(); 205 NOTREACHED();
208 return false; 206 return false;
209 } 207 }
210 208
211 std::string data; 209 std::string data;
212 if (!SerializeData(&data)) 210 if (!SerializeData(&data))
213 return false; 211 return false;
214 writer_.WriteNow(data); 212 writer_.WriteNow(data);
215 return true; 213 return true;
216 } 214 }
217 215
218 } // namespace bookmarks 216 } // namespace bookmarks
OLDNEW
« no previous file with comments | « components/bookmarks/browser/bookmark_storage.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698