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

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

Issue 379643002: Revert of 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 scoped_ptr<BookmarkLoadDetails> details, 52 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.get(), details->bb_node()); 99 AddBookmarksToIndex(details, details->bb_node());
100 AddBookmarksToIndex(details.get(), details->other_folder_node()); 100 AddBookmarksToIndex(details, details->other_folder_node());
101 AddBookmarksToIndex(details.get(), details->mobile_folder_node()); 101 AddBookmarksToIndex(details, 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.get(), extra_nodes[i]); 103 AddBookmarksToIndex(details, 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)));
111 } 110 }
112 111
113 } // namespace 112 } // namespace
114 113
115 // BookmarkLoadDetails --------------------------------------------------------- 114 // BookmarkLoadDetails ---------------------------------------------------------
116 115
117 BookmarkLoadDetails::BookmarkLoadDetails( 116 BookmarkLoadDetails::BookmarkLoadDetails(
118 BookmarkPermanentNode* bb_node, 117 BookmarkPermanentNode* bb_node,
119 BookmarkPermanentNode* other_folder_node, 118 BookmarkPermanentNode* other_folder_node,
120 BookmarkPermanentNode* mobile_folder_node, 119 BookmarkPermanentNode* mobile_folder_node,
(...skipping 35 matching lines...) Expand 10 before | Expand all | Expand 10 after
156 } 155 }
157 156
158 BookmarkStorage::~BookmarkStorage() { 157 BookmarkStorage::~BookmarkStorage() {
159 if (writer_.HasPendingWrite()) 158 if (writer_.HasPendingWrite())
160 writer_.DoScheduledWrite(); 159 writer_.DoScheduledWrite();
161 } 160 }
162 161
163 void BookmarkStorage::LoadBookmarks( 162 void BookmarkStorage::LoadBookmarks(
164 scoped_ptr<BookmarkLoadDetails> details, 163 scoped_ptr<BookmarkLoadDetails> details,
165 const scoped_refptr<base::SequencedTaskRunner>& task_runner) { 164 const scoped_refptr<base::SequencedTaskRunner>& task_runner) {
165 DCHECK(!details_.get());
166 DCHECK(details);
167 details_ = details.Pass();
166 sequenced_task_runner_->PostTask(FROM_HERE, 168 sequenced_task_runner_->PostTask(FROM_HERE,
167 base::Bind(&LoadCallback, 169 base::Bind(&LoadCallback,
168 writer_.path(), 170 writer_.path(),
169 weak_factory_.GetWeakPtr(), 171 weak_factory_.GetWeakPtr(),
170 base::Passed(&details), 172 details_.get(),
171 task_runner)); 173 task_runner));
172 } 174 }
173 175
174 void BookmarkStorage::ScheduleSave() { 176 void BookmarkStorage::ScheduleSave() {
175 writer_.ScheduleWrite(this); 177 writer_.ScheduleWrite(this);
176 } 178 }
177 179
178 void BookmarkStorage::BookmarkModelDeleted() { 180 void BookmarkStorage::BookmarkModelDeleted() {
179 // We need to save now as otherwise by the time SaveNow is invoked 181 // We need to save now as otherwise by the time SaveNow is invoked
180 // the model is gone. 182 // the model is gone.
181 if (writer_.HasPendingWrite()) 183 if (writer_.HasPendingWrite())
182 SaveNow(); 184 SaveNow();
183 model_ = NULL; 185 model_ = NULL;
184 } 186 }
185 187
186 bool BookmarkStorage::SerializeData(std::string* output) { 188 bool BookmarkStorage::SerializeData(std::string* output) {
187 BookmarkCodec codec; 189 BookmarkCodec codec;
188 scoped_ptr<base::Value> value(codec.Encode(model_)); 190 scoped_ptr<base::Value> value(codec.Encode(model_));
189 JSONStringValueSerializer serializer(output); 191 JSONStringValueSerializer serializer(output);
190 serializer.set_pretty_print(true); 192 serializer.set_pretty_print(true);
191 return serializer.Serialize(*(value.get())); 193 return serializer.Serialize(*(value.get()));
192 } 194 }
193 195
194 void BookmarkStorage::OnLoadFinished(scoped_ptr<BookmarkLoadDetails> details) { 196 void BookmarkStorage::OnLoadFinished() {
195 if (!model_) 197 if (!model_)
196 return; 198 return;
197 199
198 model_->DoneLoading(details.Pass()); 200 model_->DoneLoading(details_.Pass());
199 } 201 }
200 202
201 bool BookmarkStorage::SaveNow() { 203 bool BookmarkStorage::SaveNow() {
202 if (!model_ || !model_->loaded()) { 204 if (!model_ || !model_->loaded()) {
203 // We should only get here if we have a valid model and it's finished 205 // We should only get here if we have a valid model and it's finished
204 // loading. 206 // loading.
205 NOTREACHED(); 207 NOTREACHED();
206 return false; 208 return false;
207 } 209 }
208 210
209 std::string data; 211 std::string data;
210 if (!SerializeData(&data)) 212 if (!SerializeData(&data))
211 return false; 213 return false;
212 writer_.WriteNow(data); 214 writer_.WriteNow(data);
213 return true; 215 return true;
214 } 216 }
215 217
216 } // namespace bookmarks 218 } // 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