Commit 5b790052 authored by Olivier Li's avatar Olivier Li Committed by Commit Bot

Update comments after enabling HistoryServiceUsesTaskScheduler by default

This is the comments half of https://crrev.com/c/1960467 that was
reverted because of tests breakage. The actual flag flip is at
https://crrev.com/c/2000798. The tests have been fixed and this is conceptually
a reland.

Bug: 661143
Change-Id: Ic009bd84a8b4fe09f60967b90ff1434931e4734a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1999228Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Oliver Li <olivierli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#731654}
parent 6bd237ca
...@@ -2,12 +2,12 @@ ...@@ -2,12 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// The history system runs on a background thread so that potentially slow // The history system runs on a background sequence so that potentially slow
// database operations don't delay the browser. This backend processing is // database operations don't delay the browser. This backend processing is
// represented by HistoryBackend. The HistoryService's job is to dispatch to // represented by HistoryBackend. The HistoryService's job is to dispatch to
// that thread. // that thread.
// //
// Main thread History thread // Main thread backend_task_runner_
// ----------- -------------- // ----------- --------------
// HistoryService <----------------> HistoryBackend // HistoryService <----------------> HistoryBackend
// -> HistoryDatabase // -> HistoryDatabase
......
...@@ -468,7 +468,7 @@ class HistoryService : public KeyedService { ...@@ -468,7 +468,7 @@ class HistoryService : public KeyedService {
// Generic Stuff ------------------------------------------------------------- // Generic Stuff -------------------------------------------------------------
// Schedules a HistoryDBTask for running on the history backend thread. See // Schedules a HistoryDBTask for running on the history backend. See
// HistoryDBTask for details on what this does. Takes ownership of |task|. // HistoryDBTask for details on what this does. Takes ownership of |task|.
virtual base::CancelableTaskTracker::TaskId ScheduleDBTask( virtual base::CancelableTaskTracker::TaskId ScheduleDBTask(
const base::Location& from_here, const base::Location& from_here,
...@@ -493,15 +493,15 @@ class HistoryService : public KeyedService { ...@@ -493,15 +493,15 @@ class HistoryService : public KeyedService {
// Testing ------------------------------------------------------------------- // Testing -------------------------------------------------------------------
// Runs |flushed| after bouncing off the history thread. // Runs |flushed| after the backend has processed all other pre-existing
// tasks.
void FlushForTest(base::OnceClosure flushed); void FlushForTest(base::OnceClosure flushed);
// Designed for unit tests, this passes the given task on to the history // Designed for unit tests, this passes the given task on to the history
// backend to be called once the history backend has terminated. This allows // backend to be called once the history backend has terminated. This allows
// callers to know when the history thread is complete and the database files // callers to know when the history backend has been safely deleted and the
// can be deleted and the next test run. Otherwise, the history thread may // database files can be deleted and the next test run.
// still be running, causing problems in subsequent tests.
//
// There can be only one closing task, so this will override any previously // There can be only one closing task, so this will override any previously
// set task. We will take ownership of the pointer and delete it when done. // set task. We will take ownership of the pointer and delete it when done.
// The task will be run on the calling thread (this function is threadsafe). // The task will be run on the calling thread (this function is threadsafe).
...@@ -511,9 +511,9 @@ class HistoryService : public KeyedService { ...@@ -511,9 +511,9 @@ class HistoryService : public KeyedService {
// into the database. This assumes the URL doesn't exist in the database // into the database. This assumes the URL doesn't exist in the database
// //
// Calling this function many times may be slow because each call will // Calling this function many times may be slow because each call will
// dispatch to the history thread and will be a separate database // post a separate database transaction in a task. If this functionality
// transaction. If this functionality is needed for importing many URLs, // is needed for importing many URLs, callers should use AddPagesWithDetails()
// callers should use AddPagesWithDetails() instead. // instead.
// //
// Note that this routine (and AddPageWithDetails()) always adds a single // Note that this routine (and AddPageWithDetails()) always adds a single
// visit using the |last_visit| timestamp, and a PageTransition type of LINK, // visit using the |last_visit| timestamp, and a PageTransition type of LINK,
...@@ -594,9 +594,9 @@ class HistoryService : public KeyedService { ...@@ -594,9 +594,9 @@ class HistoryService : public KeyedService {
// that is only set by unittests which causes the backend to not init its DB. // that is only set by unittests which causes the backend to not init its DB.
bool Init(bool no_db, const HistoryDatabaseParams& history_database_params); bool Init(bool no_db, const HistoryDatabaseParams& history_database_params);
// Called by the HistoryURLProvider class to schedule an autocomplete, it // Called by the HistoryURLProvider class to schedule an autocomplete, it will
// will be called back on the internal history thread with the history // be called back with the history database so it can query. See
// database so it can query. See history_url_provider.h for a diagram. // history_url_provider.h for a diagram.
void ScheduleAutocomplete( void ScheduleAutocomplete(
base::OnceCallback<void(HistoryBackend*, URLDatabase*)> callback); base::OnceCallback<void(HistoryBackend*, URLDatabase*)> callback);
...@@ -834,8 +834,8 @@ class HistoryService : public KeyedService { ...@@ -834,8 +834,8 @@ class HistoryService : public KeyedService {
void NotifyProfileError(sql::InitStatus init_status, void NotifyProfileError(sql::InitStatus init_status,
const std::string& diagnostics); const std::string& diagnostics);
// Call to schedule a given task for running on the history thread with the // Call to post a given task for running on the history backend sequence with
// specified priority. The task will have ownership taken. // the specified priority. The task will have ownership taken.
void ScheduleTask(SchedulePriority priority, base::OnceClosure task); void ScheduleTask(SchedulePriority priority, base::OnceClosure task);
// Called when the favicons for the given page URLs (e.g. // Called when the favicons for the given page URLs (e.g.
...@@ -858,17 +858,16 @@ class HistoryService : public KeyedService { ...@@ -858,17 +858,16 @@ class HistoryService : public KeyedService {
// Cleanup() is called. // Cleanup() is called.
scoped_refptr<base::SequencedTaskRunner> backend_task_runner_; scoped_refptr<base::SequencedTaskRunner> backend_task_runner_;
// This class has most of the implementation and runs on the 'thread_'. // This class has most of the implementation. You MUST communicate with this
// You MUST communicate with this class ONLY through the thread_'s // class ONLY through |backend_task_runner_|.
// task_runner().
// //
// This pointer will be null once Cleanup() has been called, meaning no // This pointer will be null once Cleanup() has been called, meaning no
// more calls should be made to the history thread. // more tasks should be scheduled.
scoped_refptr<HistoryBackend> history_backend_; scoped_refptr<HistoryBackend> history_backend_;
// A cache of the user-typed URLs kept in memory that is used by the // A cache of the user-typed URLs kept in memory that is used by the
// autocomplete system. This will be null until the database has been created // autocomplete system. This will be null until the database has been created
// on the background thread. // in the backend.
// TODO(mrossetti): Consider changing ownership. See http://crbug.com/138321 // TODO(mrossetti): Consider changing ownership. See http://crbug.com/138321
std::unique_ptr<InMemoryHistoryBackend> in_memory_backend_; std::unique_ptr<InMemoryHistoryBackend> in_memory_backend_;
......
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