Skip to content

Commit

Permalink
Enable HistoryServiceUsesTaskScheduler by default
Browse files Browse the repository at this point in the history
Change-Id: Iaee9edf31fd2303a28bbeedc155cb7dd9381cdb6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1960467
Auto-Submit: Oliver Li <olivierli@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Oliver Li <olivierli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728929}
  • Loading branch information
Olivier Li authored and Commit Bot committed Jan 7, 2020
1 parent ad0faec commit 738d6a1
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 22 deletions.
6 changes: 3 additions & 3 deletions components/history/core/browser/history_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// 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
// represented by HistoryBackend. The HistoryService's job is to dispatch to
// that thread.
//
// Main thread History thread
// Main thread backend_task_runner_
// ----------- --------------
// HistoryService <----------------> HistoryBackend
// -> HistoryDatabase
Expand Down Expand Up @@ -71,7 +71,7 @@ const char* kHistoryThreadName = "Chrome_HistoryThread";

// static
const base::Feature HistoryService::kHistoryServiceUsesTaskScheduler{
"HistoryServiceUsesTaskScheduler", base::FEATURE_DISABLED_BY_DEFAULT};
"HistoryServiceUsesTaskScheduler", base::FEATURE_ENABLED_BY_DEFAULT};

// Sends messages from the backend to us on the main thread. This must be a
// separate class from the history service so that it can hold a reference to
Expand Down
37 changes: 18 additions & 19 deletions components/history/core/browser/history_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ class HistoryService : public KeyedService {

// 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|.
virtual base::CancelableTaskTracker::TaskId ScheduleDBTask(
const base::Location& from_here,
Expand All @@ -493,15 +493,15 @@ class HistoryService : public KeyedService {

// 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);

// 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
// callers to know when the history thread is complete and the database files
// can be deleted and the next test run. Otherwise, the history thread may
// still be running, causing problems in subsequent tests.
//
// callers to know when the history backend has been safely deleted and the
// database files can be deleted and the next test run.

// 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.
// The task will be run on the calling thread (this function is threadsafe).
Expand All @@ -511,9 +511,9 @@ class HistoryService : public KeyedService {
// 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
// dispatch to the history thread and will be a separate database
// transaction. If this functionality is needed for importing many URLs,
// callers should use AddPagesWithDetails() instead.
// post a separate database transaction in a task. If this functionality
// is needed for importing many URLs, callers should use AddPagesWithDetails()
// instead.
//
// Note that this routine (and AddPageWithDetails()) always adds a single
// visit using the |last_visit| timestamp, and a PageTransition type of LINK,
Expand Down Expand Up @@ -594,9 +594,9 @@ class HistoryService : public KeyedService {
// 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);

// Called by the HistoryURLProvider class to schedule an autocomplete, it
// will be called back on the internal history thread with the history
// database so it can query. See history_url_provider.h for a diagram.
// Called by the HistoryURLProvider class to schedule an autocomplete, it will
// be called back with the history database so it can query. See
// history_url_provider.h for a diagram.
void ScheduleAutocomplete(
base::OnceCallback<void(HistoryBackend*, URLDatabase*)> callback);

Expand Down Expand Up @@ -834,8 +834,8 @@ class HistoryService : public KeyedService {
void NotifyProfileError(sql::InitStatus init_status,
const std::string& diagnostics);

// Call to schedule a given task for running on the history thread with the
// specified priority. The task will have ownership taken.
// Call to post a given task for running on the history backend sequence with
// the specified priority. The task will have ownership taken.
void ScheduleTask(SchedulePriority priority, base::OnceClosure task);

// Called when the favicons for the given page URLs (e.g.
Expand All @@ -858,17 +858,16 @@ class HistoryService : public KeyedService {
// Cleanup() is called.
scoped_refptr<base::SequencedTaskRunner> backend_task_runner_;

// This class has most of the implementation and runs on the 'thread_'.
// You MUST communicate with this class ONLY through the thread_'s
// task_runner().
// This class has most of the implementation. You MUST communicate with this
// class ONLY through |backend_task_runner_|.
//
// 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_;

// 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
// on the background thread.
// in the backend.
// TODO(mrossetti): Consider changing ownership. See http://crbug.com/138321
std::unique_ptr<InMemoryHistoryBackend> in_memory_backend_;

Expand Down

0 comments on commit 738d6a1

Please sign in to comment.