history_api -> UIThreadExtensionFunction.

chrome/browser/extensions/api/history/history_api.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/history/history_api.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 201 matching lines @@
 }
 
 void HistoryAPI::OnListenerAdded(const EventListenerInfo& details) {
   Profile* profile = Profile::FromBrowserContext(browser_context_);
   history_event_router_.reset(new HistoryEventRouter(
       profile, HistoryServiceFactory::GetForProfile(
                    profile, ServiceAccessType::EXPLICIT_ACCESS)));
   EventRouter::Get(browser_context_)->UnregisterObserver(this);
 }
 
-bool HistoryFunction::ValidateUrl(const std::string& url_string, GURL* url) {
+bool HistoryFunction::ValidateUrl(const std::string& url_string,
+                                  GURL* url,
+                                  std::string* error) {
   GURL temp_url(url_string);
   if (!temp_url.is_valid()) {
-    error_ = kInvalidUrlError;
+    *error = kInvalidUrlError;
     return false;
   }
   url->Swap(&temp_url);
   return true;
 }
 
-bool HistoryFunction::VerifyDeleteAllowed() {
+bool HistoryFunction::VerifyDeleteAllowed(std::string* error) {
   PrefService* prefs = GetProfile()->GetPrefs();
   if (!prefs->GetBoolean(prefs::kAllowDeletingBrowserHistory)) {
-    error_ = kDeleteProhibitedError;
+    *error = kDeleteProhibitedError;
     return false;
   }
   return true;
 }
 
 base::Time HistoryFunction::GetTime(double ms_from_epoch) {
   // The history service has seconds resolution, while javascript Date() has
   // milliseconds resolution.
   double seconds_from_epoch = ms_from_epoch / 1000.0;
   // Time::FromDoubleT converts double time 0 to empty Time object. So we need
   // to do special handling here.
   return (seconds_from_epoch == 0) ?
       base::Time::UnixEpoch() : base::Time::FromDoubleT(seconds_from_epoch);
 }
 
-HistoryFunctionWithCallback::HistoryFunctionWithCallback() {
+Profile* HistoryFunction::GetProfile() const {
+  return Profile::FromBrowserContext(browser_context());
 }
 
-HistoryFunctionWithCallback::~HistoryFunctionWithCallback() {
+HistoryFunctionWithCallback::HistoryFunctionWithCallback() {}
+
+HistoryFunctionWithCallback::~HistoryFunctionWithCallback() {}
+
+void HistoryFunctionWithCallback::SendAsyncResponse(

[Devlin, 2017/02/03 22:30:53]

Do we have to SendAsyncResponse in order to prevent accidentally calling back
synchronously?  It looks like most of these are chained from callbacks - are
some of those (possibly) called synchronously?

[lazyboy, 2017/02/03 23:49:06]

HistoryService apis run on its own TaskRunner, e.g.:
https://cs.chromium.org/chromium/src/components/history/core/browser/history_...

All of these apis
QueryURL/QueryHistory/AddPage/DeleteURL/ExpireHistoryBetween
will call PostTask, so the callbacks shouldn't ever be called synchronously.

I discovered one early out in AddPage from HistoryAddUrlFunction:
https://cs.chromium.org/chromium/src/components/history/core/browser/history_...
However, since we don't wait for the task to finish before responding, that
shouldn't be an issue either.

[Devlin, 2017/02/06 16:09:26]

So if that's the case (that all these methods on history service run async), do
we need SendAsyncResponse at all?  Or could we just use Respond() in each place?

[lazyboy, 2017/02/14 00:55:30]

Done.

+    ResponseValue response_value) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::Bind(&HistoryFunctionWithCallback::SendResponseToCallback, this,
+                 base::Passed(&response_value)));
 }
 
-bool HistoryFunctionWithCallback::RunAsync() {
-  AddRef();  // Balanced in SendAysncRepose() and below.
-  bool retval = RunAsyncImpl();
-  if (false == retval)
-    Release();
-  return retval;
+void HistoryFunctionWithCallback::SendResponseToCallback(
+    ResponseValue response_value) {
+  Respond(std::move(response_value));
+  Release();  // Balanced in Run().
 }
 
-void HistoryFunctionWithCallback::SendAsyncResponse() {
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&HistoryFunctionWithCallback::SendResponseToCallback, this));
-}
-
-void HistoryFunctionWithCallback::SendResponseToCallback() {
-  SendResponse(true);
-  Release();  // Balanced in RunAsync().
-}
-
-bool HistoryGetVisitsFunction::RunAsyncImpl() {
+ExtensionFunction::ResponseAction HistoryGetVisitsFunction::Run() {
   std::unique_ptr<GetVisits::Params> params(GetVisits::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   GURL url;
-  if (!ValidateUrl(params->details.url, &url))
-    return false;
+  std::string error;
+  if (!ValidateUrl(params->details.url, &url, &error))
+    return RespondNow(Error(error));
 
   history::HistoryService* hs = HistoryServiceFactory::GetForProfile(
       GetProfile(), ServiceAccessType::EXPLICIT_ACCESS);
   hs->QueryURL(url,
                true,  // Retrieve full history of a URL.
                base::Bind(&HistoryGetVisitsFunction::QueryComplete,
                           base::Unretained(this)),
                &task_tracker_);
-  return true;
+  AddRef();               // Balanced in SendAsyncResponse().
+  return RespondLater();  // QueryComplete() will be called asynchronously.
 }
 
 void HistoryGetVisitsFunction::QueryComplete(
     bool success,
     const history::URLRow& url_row,
     const history::VisitVector& visits) {
   VisitItemList visit_item_vec;
   if (success && !visits.empty()) {
     for (const history::VisitRow& visit : visits)
       visit_item_vec.push_back(GetVisitItem(visit));
   }
 
-  results_ = GetVisits::Results::Create(visit_item_vec);
-  SendAsyncResponse();
+  SendAsyncResponse(ArgumentList(GetVisits::Results::Create(visit_item_vec)));

[Devlin, 2017/02/06 16:09:26]

e.g. this becomes just:
Respond(ArgumentList(...));
Release();  // Balanced in Run().

[lazyboy, 2017/02/14 00:55:30]

Done.

 }
 
-bool HistorySearchFunction::RunAsyncImpl() {
+ExtensionFunction::ResponseAction HistorySearchFunction::Run() {
   std::unique_ptr<Search::Params> params(Search::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   base::string16 search_text = base::UTF8ToUTF16(params->query.text);
 
   history::QueryOptions options;
   options.SetRecentDayRange(1);
   options.max_count = 100;
 
   if (params->query.start_time.get())
     options.begin_time = GetTime(*params->query.start_time);
   if (params->query.end_time.get())
     options.end_time = GetTime(*params->query.end_time);
   if (params->query.max_results.get())
     options.max_count = *params->query.max_results;
 
   history::HistoryService* hs = HistoryServiceFactory::GetForProfile(
       GetProfile(), ServiceAccessType::EXPLICIT_ACCESS);
   hs->QueryHistory(search_text,
                    options,
                    base::Bind(&HistorySearchFunction::SearchComplete,
                               base::Unretained(this)),
                    &task_tracker_);
 
-  return true;
+  AddRef();               // Balanced in SendAsyncResponse().
+  return RespondLater();  // SearchComplete() will be called asynchronously.
 }
 
 void HistorySearchFunction::SearchComplete(history::QueryResults* results) {
   HistoryItemList history_item_vec;
   if (results && !results->empty()) {
     for (const history::URLResult* item : *results)
       history_item_vec.push_back(GetHistoryItem(*item));
   }
-  results_ = Search::Results::Create(history_item_vec);
-  SendAsyncResponse();
+  SendAsyncResponse(ArgumentList(Search::Results::Create(history_item_vec)));
 }
 
-bool HistoryAddUrlFunction::RunAsync() {
+ExtensionFunction::ResponseAction HistoryAddUrlFunction::Run() {
   std::unique_ptr<AddUrl::Params> params(AddUrl::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   GURL url;
-  if (!ValidateUrl(params->details.url, &url))
-    return false;
+  std::string error;
+  if (!ValidateUrl(params->details.url, &url, &error))
+    return RespondNow(Error(error));
 
   history::HistoryService* hs = HistoryServiceFactory::GetForProfile(
       GetProfile(), ServiceAccessType::EXPLICIT_ACCESS);
   hs->AddPage(url, base::Time::Now(), history::SOURCE_EXTENSION);
 
-  SendResponse(true);
-  return true;
+  return RespondNow(NoArguments());
 }
 
-bool HistoryDeleteUrlFunction::RunAsync() {
+ExtensionFunction::ResponseAction HistoryDeleteUrlFunction::Run() {
   std::unique_ptr<DeleteUrl::Params> params(DeleteUrl::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
-  if (!VerifyDeleteAllowed())
-    return false;
+  std::string error;
+  if (!VerifyDeleteAllowed(&error))
+    return RespondNow(Error(error));
 
   GURL url;
-  if (!ValidateUrl(params->details.url, &url))
-    return false;
+  if (!ValidateUrl(params->details.url, &url, &error))
+    return RespondNow(Error(error));
 
   history::HistoryService* hs = HistoryServiceFactory::GetForProfile(
       GetProfile(), ServiceAccessType::EXPLICIT_ACCESS);
   hs->DeleteURL(url);
 
   // Also clean out from the activity log. If the activity log testing flag is
   // set then don't clean so testers can see what potentially malicious
   // extensions have been trying to clean from their logs.
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableExtensionActivityLogTesting)) {
     ActivityLog* activity_log = ActivityLog::GetInstance(GetProfile());
     DCHECK(activity_log);
     activity_log->RemoveURL(url);
   }
 
-  SendResponse(true);
-  return true;
+  return RespondNow(NoArguments());
 }
 
-bool HistoryDeleteRangeFunction::RunAsyncImpl() {
+ExtensionFunction::ResponseAction HistoryDeleteRangeFunction::Run() {
   std::unique_ptr<DeleteRange::Params> params(
       DeleteRange::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
-  if (!VerifyDeleteAllowed())
-    return false;
+  std::string error;
+  if (!VerifyDeleteAllowed(&error))
+    return RespondNow(Error(error));
 
   base::Time start_time = GetTime(params->range.start_time);
   base::Time end_time = GetTime(params->range.end_time);
 
   std::set<GURL> restrict_urls;
   history::HistoryService* hs = HistoryServiceFactory::GetForProfile(
       GetProfile(), ServiceAccessType::EXPLICIT_ACCESS);
   hs->ExpireHistoryBetween(
       restrict_urls,
       start_time,
       end_time,
       base::Bind(&HistoryDeleteRangeFunction::DeleteComplete,
                  base::Unretained(this)),
       &task_tracker_);
 
   // Also clean from the activity log unless in testing mode.
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableExtensionActivityLogTesting)) {
     ActivityLog* activity_log = ActivityLog::GetInstance(GetProfile());
     DCHECK(activity_log);
     activity_log->RemoveURLs(restrict_urls);
   }
 
-  return true;
+  AddRef();               // Balanced in SendAsyncResponse().
+  return RespondLater();  // DeleteComplete() will be called asynchronously.
 }
 
 void HistoryDeleteRangeFunction::DeleteComplete() {
-  SendAsyncResponse();
+  SendAsyncResponse(NoArguments());
 }
 
-bool HistoryDeleteAllFunction::RunAsyncImpl() {
-  if (!VerifyDeleteAllowed())
-    return false;
+ExtensionFunction::ResponseAction HistoryDeleteAllFunction::Run() {
+  std::string error;
+  if (!VerifyDeleteAllowed(&error))
+    return RespondNow(Error(error));
 
   std::set<GURL> restrict_urls;
   history::HistoryService* hs = HistoryServiceFactory::GetForProfile(
       GetProfile(), ServiceAccessType::EXPLICIT_ACCESS);
   hs->ExpireHistoryBetween(
       restrict_urls,
       base::Time(),      // Unbounded beginning...
       base::Time(),      // ...and the end.
       base::Bind(&HistoryDeleteAllFunction::DeleteComplete,
                  base::Unretained(this)),
       &task_tracker_);
 
   // Also clean from the activity log unless in testing mode.
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableExtensionActivityLogTesting)) {
     ActivityLog* activity_log = ActivityLog::GetInstance(GetProfile());
     DCHECK(activity_log);
     activity_log->RemoveURLs(restrict_urls);
   }
 
-  return true;
+  AddRef();               // Balanced in SendAsyncResponse().
+  return RespondLater();  // DeleteComplete() will be called asynchronously.
 }
 
 void HistoryDeleteAllFunction::DeleteComplete() {
-  SendAsyncResponse();
+  SendAsyncResponse(NoArguments());
 }
 
 }  // namespace extensions