fix race condition and assumption of async url loading

BUILD.gn

@@ -27,6 +27,7 @@ source_set("bat-native-ledger") {
     "include/bat/ledger/ledger.h",
     "include/bat/ledger/ledger_callback_handler.h",
     "include/bat/ledger/ledger_client.h",
+    "include/bat/ledger/ledger_url_loader.h",
     "include/bat/ledger/ledger_task_runner.h",
     "src/bat/ledger/ledger.cc",
     "src/bat_client.cc",

include/bat/ledger/ledger_client.h

@@ -9,6 +9,7 @@
 
 #include "bat/ledger/ledger_callback_handler.h"
 #include "bat/ledger/ledger_task_runner.h"
+#include "bat/ledger/ledger_url_loader.h"
 
 namespace ledger {
 
@@ -33,12 +34,12 @@ class LedgerClient {
                                LedgerCallbackHandler* handler) = 0;
   virtual void SavePublisherState(const std::string& publisher_state,
                                   LedgerCallbackHandler* handler) = 0;
-  virtual uint64_t LoadURL(const std::string& url,
-                       const std::vector<std::string>& headers,
-                       const std::string& content,
-                       const std::string& contentType,
-                       const URL_METHOD& method,
-                       ledger::LedgerCallbackHandler* handler) = 0;
+  virtual std::unique_ptr<ledger::LedgerURLLoader> LoadURL(const std::string& url,
+      const std::vector<std::string>& headers,
+      const std::string& content,
+      const std::string& contentType,
+      const ledger::URL_METHOD& method,
+      ledger::LedgerCallbackHandler* handler) = 0;
   // RunIOTask and RunTask are temporary workarounds for leveldb
   // and we should replace them with a ledger_client api for reading/writing
   // individual records

include/bat/ledger/ledger_task_runner.h

@@ -2,8 +2,8 @@
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#ifndef BAT_LEDGER_LEDGER_IO_TASK_RUNNER_
-#define BAT_LEDGER_LEDGER_IO_TASK_RUNNER_
+#ifndef BAT_LEDGER_LEDGER_TASK_RUNNER_
+#define BAT_LEDGER_LEDGER_TASK_RUNNER_
 
 #include <string>
 
@@ -17,4 +17,4 @@ class LedgerTaskRunner {
 
 }  // namespace ledger
 
-#endif  // BAT_LEDGER_LEDGER_IO_TASK_RUNNER_
+#endif  // BAT_LEDGER_LEDGER_TASK_RUNNER_

include/bat/ledger/ledger_url_loader.h

@@ -0,0 +1,21 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+* License, v. 2.0. If a copy of the MPL was not distributed with this
+* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef BAT_LEDGER_LEDGER_URL_LOADER_
+#define BAT_LEDGER_LEDGER_URL_LOADER_
+
+#include <string>
+
+namespace ledger {
+
+class LedgerURLLoader {
+ public:
+  virtual ~LedgerURLLoader() = default;
+  virtual void Start() = 0;
+  virtual uint64_t request_id() = 0;
+};
+
+}  // namespace ledger
+
+#endif  // BAT_LEDGER_LEDGER_URL_LOADER_

src/bat_client.cc

@@ -79,7 +79,7 @@ void BatClient::registerPersona() {
   auto request_id = ledger_->LoadURL(buildURL(REGISTER_PERSONA, PREFIX_V2),
       std::vector<std::string>(), "", "",
       ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::requestCredentialsCallback,
                                        this,
                                        _1,
@@ -139,7 +139,7 @@ void BatClient::requestCredentialsCallback(bool result, const std::string& respo
     buildURL((std::string)REGISTER_PERSONA + "/" + state_->userId_, PREFIX_V2),
     headers, payloadStringify, "application/json; charset=utf-8",
     ledger::URL_METHOD::POST, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::registerPersonaCallback,
                                        this,
                                        _1,
@@ -202,7 +202,7 @@ void BatClient::publisherTimestamp( bool save_state) {
 
   auto request_id = ledger_->LoadURL(buildURL(PUBLISHER_TIMESTAMP, PREFIX_V3), std::vector<std::string>(), "", "",
     ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::publisherTimestampCallback,
                                        this,
                                        _1,
@@ -231,8 +231,9 @@ uint64_t BatClient::getPublisherTimestamp() {
   return publisherTimestamp_;
 }
 
-uint64_t BatClient::publisherInfo(const std::string& publisher,
-                             ledger::LedgerCallbackHandler* handler){
+std::unique_ptr<ledger::LedgerURLLoader> BatClient::publisherInfo(
+    const std::string& publisher,
+    ledger::LedgerCallbackHandler* handler){
   return ledger_->LoadURL(buildURL(PUBLISHER_INFO + publisher, PREFIX_V3),
       std::vector<std::string>(), "", "", ledger::URL_METHOD::GET, handler);
 }
@@ -266,7 +267,7 @@ const std::string& BatClient::getLTCAddress() const {
 //       "",
 //       "",
 //       ledger::URL_METHOD::GET, &handler_);
-//   handler_.AddRequestHandler(request_id,
+//   handler_.AddRequestHandler(std::move(request_id),
 //                              std::bind(&BatClient::publisherTimestampCallback,
 //                                        this,
 //                                        _1,
@@ -288,7 +289,7 @@ void BatClient::reconcile(const std::string& viewingId) {
       ledger::URL_METHOD::GET,
       &handler_);
 
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::reconcileCallback,
                                        this,
                                        _1,
@@ -315,7 +316,7 @@ void BatClient::currentReconcile() {
   auto request_id = ledger_->LoadURL(buildURL(path, ""),
       std::vector<std::string>(), "", "",
       ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::currentReconcileCallback,
                                        this,
                                        _1,
@@ -379,7 +380,7 @@ void BatClient::currentReconcileCallback(bool result, const std::string& respons
     headers, payloadStringify, "application/json; charset=utf-8",
     ledger::URL_METHOD::PUT,
     &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::reconcilePayloadCallback,
                                        this,
                                        _1,
@@ -412,7 +413,7 @@ void BatClient::reconcilePayloadCallback(bool result, const std::string& respons
 
   auto request_id = ledger_->LoadURL(buildURL(UPDATE_RULES_V1, ""),
     std::vector<std::string>(), "", "", ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::updateRulesCallback,
                                        this,
                                        _1,
@@ -431,7 +432,7 @@ void BatClient::updateRulesCallback(bool result, const std::string& response, co
   auto request_id = ledger_->LoadURL(buildURL(UPDATE_RULES_V2, ""),
     std::vector<std::string>(), "", "",
     ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::updateRulesV2Callback,
                                        this,
                                        _1,
@@ -458,7 +459,7 @@ void BatClient::registerViewing() {
   auto request_id = ledger_->LoadURL(buildURL((std::string)REGISTER_VIEWING, PREFIX_V2),
     std::vector<std::string>(), "", "",
     ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::registerViewingCallback,
                                        this,
                                        _1,
@@ -490,7 +491,7 @@ void BatClient::viewingCredentials(const std::string& proofStringified, const st
   auto request_id = ledger_->LoadURL(buildURL((std::string)REGISTER_VIEWING + "/" + anonizeViewingId, PREFIX_V2),
     std::vector<std::string>(), proofStringified, "application/json; charset=utf-8",
     ledger::URL_METHOD::POST, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::viewingCredentialsCallback,
                                        this,
                                        _1,
@@ -631,7 +632,7 @@ void BatClient::prepareBatch(const braveledger_bat_helper::BALLOT_ST& ballot, co
 
   auto request_id = ledger_->LoadURL(buildURL((std::string)SURVEYOR_BATCH_VOTING + transaction.anonizeViewingId_, PREFIX_V2),
     std::vector<std::string>(), "", "", ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::prepareBatchCallback,
                                        this,
                                        _1,
@@ -729,7 +730,7 @@ void BatClient::prepareBallot(const braveledger_bat_helper::BALLOT_ST& ballot, c
 
   auto request_id = ledger_->LoadURL(buildURL((std::string)SURVEYOR_VOTING + surveyorIdEncoded + "/" + transaction.anonizeViewingId_, PREFIX_V2),
     std::vector<std::string>(), "", "", ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::prepareBallotCallback,
                                        this,
                                        _1,
@@ -800,7 +801,7 @@ void BatClient::commitBallot(const braveledger_bat_helper::BALLOT_ST& ballot, co
 
   auto request_id = ledger_->LoadURL(buildURL((std::string)SURVEYOR_VOTING + surveyorIdEncoded, PREFIX_V2),
     std::vector<std::string>(), payload, "", ledger::URL_METHOD::PUT, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::commitBallotCallback,
                                        this,
                                        _1,
@@ -862,7 +863,7 @@ void BatClient::recoverWallet(const std::string& passPhrase) {
   auto request_id = ledger_->LoadURL(buildURL((std::string)RECOVER_WALLET_PUBLIC_KEY + publicKeyHex, ""),
     std::vector<std::string>(), "", "",
     ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::recoverWalletPublicKeyCallback,
                                        this,
                                        _1,
@@ -877,7 +878,7 @@ void BatClient::recoverWalletPublicKeyCallback(bool result, const std::string& r
 
   auto request_id = ledger_->LoadURL(buildURL((std::string)RECOVER_WALLET + recoveryId, ""),
     std::vector<std::string>(), "", "", ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::recoverWalletCallback,
                                        this,
                                        _1,
@@ -911,7 +912,7 @@ void BatClient::getPromotion(const std::string& lang, const std::string& forPaym
 
   auto request_id = ledger_->LoadURL(buildURL((std::string)GET_SET_PROMOTION + arguments, ""),
     std::vector<std::string>(), "", "", ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::getPromotionCallback,
                                        this,
                                        _1,
@@ -929,7 +930,7 @@ void BatClient::setPromotion(const std::string& promotionId, const std::string&
 
   auto request_id = ledger_->LoadURL(buildURL((std::string)GET_SET_PROMOTION + "/" + state_->walletInfo_.paymentId_, ""),
     std::vector<std::string>(), payload, "", ledger::URL_METHOD::PUT, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::setPromotionCallback,
                                        this,
                                        _1,
@@ -944,7 +945,7 @@ void BatClient::getPromotionCaptcha() {
   auto request_id = ledger_->LoadURL(buildURL((std::string)GET_PROMOTION_CAPTCHA + state_->walletInfo_.paymentId_, ""),
     std::vector<std::string>(), "", "",
       ledger::URL_METHOD::GET, &handler_);
-  handler_.AddRequestHandler(request_id,
+  handler_.AddRequestHandler(std::move(request_id),
                              std::bind(&BatClient::getPromotionCaptchaCallback,
                                        this,
                                        _1,

src/bat_client.h

@@ -10,6 +10,7 @@
 #include <mutex>
 
 #include "bat/ledger/ledger_callback_handler.h"
+#include "bat/ledger/ledger_url_loader.h"
 #include "bat_helper.h"
 #include "url_request_handler.h"
 
@@ -29,8 +30,9 @@ class BatClient : public ledger::LedgerCallbackHandler {
   void registerPersonaCallback(bool result, const std::string& response, const braveledger_bat_helper::FETCH_CALLBACK_EXTRA_DATA_ST& extraData);
   void publisherTimestampCallback(bool result, const std::string& response, const braveledger_bat_helper::FETCH_CALLBACK_EXTRA_DATA_ST& extra_data);
   uint64_t getPublisherTimestamp();
-  uint64_t publisherInfo(const std::string& publisher,
-                        ledger::LedgerCallbackHandler* handler);
+  std::unique_ptr<ledger::LedgerURLLoader> publisherInfo(
+      const std::string& publisher,
+      ledger::LedgerCallbackHandler* handler);
   void setContributionAmount(const double& amount);
   const std::string& getBATAddress() const;
   const std::string& getBTCAddress() const;

src/bat_get_media.cc

@@ -108,9 +108,9 @@ void BatGetMedia::getPublisherFromMediaProps(const std::string& mediaId, const s
   }
   if (YOUTUBE_MEDIA_TYPE == providerName) {
     // TODO(bridiver) - this is also an issue because we're making these calls from the IO thread
-    auto request_id = ledger_->LoadURL((std::string)YOUTUBE_PROVIDER_URL + "?format=json&url=" + mediaURLEncoded,
+    auto request = ledger_->LoadURL((std::string)YOUTUBE_PROVIDER_URL + "?format=json&url=" + mediaURLEncoded,
       std::vector<std::string>(), "", "", ledger::URL_METHOD::GET, &handler_);
-    handler_.AddRequestHandler(request_id,
+    handler_.AddRequestHandler(std::move(request),
         std::bind(&BatGetMedia::getPublisherFromMediaPropsCallback,
         this,
         _1,
@@ -256,9 +256,9 @@ void BatGetMedia::getPublisherFromMediaPropsCallback(bool result, const std::str
     newExtraData.string3 = publisherURL;
     newExtraData.string4 = publisherName;
 
-    auto request_id = ledger_->LoadURL(publisherURL,
+    auto request = ledger_->LoadURL(publisherURL,
         std::vector<std::string>(), "", "", ledger::URL_METHOD::GET, &handler_);
-    handler_.AddRequestHandler(request_id,
+    handler_.AddRequestHandler(std::move(request),
         std::bind(&BatGetMedia::getPublisherInfoCallback,
                   this,
                   _1,

src/ledger_impl.cc

@@ -65,12 +65,12 @@ void LedgerImpl::OnWalletCreated(ledger::Result result) {
   ledger_client_->OnWalletCreated(result);
 }
 
-uint64_t LedgerImpl::LoadURL(const std::string& url,
-                    const std::vector<std::string>& headers,
-                    const std::string& content,
-                    const std::string& contentType,
-                    const ledger::URL_METHOD& method,
-                    ledger::LedgerCallbackHandler* handler) {
+std::unique_ptr<ledger::LedgerURLLoader> LedgerImpl::LoadURL(const std::string& url,
+    const std::vector<std::string>& headers,
+    const std::string& content,
+    const std::string& contentType,
+    const ledger::URL_METHOD& method,
+    ledger::LedgerCallbackHandler* handler) {
   return ledger_client_->LoadURL(
       url, headers, content, contentType, method, handler);
 }
@@ -123,13 +123,14 @@ void LedgerImpl::saveVisitCallback(const std::string& publisher,
 
   // Update publisher verified or not flag
   //LOG(ERROR) << "!!!getting publisher info";
-  auto request_id = bat_client_->publisherInfo(publisher, &handler_);
-  handler_.AddRequestHandler(request_id, std::bind(&LedgerImpl::publisherInfoCallback,
-                                            this,
-                                            publisher,
-                                            publisherTimestamp,
-                                            _1,
-                                            _2));
+  auto request = bat_client_->publisherInfo(publisher, &handler_);
+  handler_.AddRequestHandler(std::move(request),
+                             std::bind(&LedgerImpl::publisherInfoCallback,
+                                        this,
+                                        publisher,
+                                        publisherTimestamp,
+                                        _1,
+                                        _2));
 }
 
 void LedgerImpl::publisherInfoCallback(const std::string& publisher,

src/ledger_impl.h

@@ -11,6 +11,7 @@
 
 #include "bat/ledger/ledger.h"
 #include "bat/ledger/ledger_client.h"
+#include "bat/ledger/ledger_url_loader.h"
 #include "bat_helper.h"
 #include "ledger_task_runner_impl.h"
 #include "url_request_handler.h"
@@ -70,12 +71,12 @@ class LedgerImpl : public ledger::Ledger,
 
   void OnWalletCreated(ledger::Result);
 
-  uint64_t LoadURL(const std::string& url,
-                       const std::vector<std::string>& headers,
-                       const std::string& content,
-                       const std::string& contentType,
-                       const ledger::URL_METHOD& method,
-                       ledger::LedgerCallbackHandler* handler);
+  std::unique_ptr<ledger::LedgerURLLoader> LoadURL(const std::string& url,
+      const std::vector<std::string>& headers,
+      const std::string& content,
+      const std::string& contentType,
+      const ledger::URL_METHOD& method,
+      ledger::LedgerCallbackHandler* handler);
   void OnReconcileComplete(ledger::Result result,
                            const std::string& viewing_id);
   void RunIOTask(LedgerTaskRunnerImpl::Task task);

src/test/mock_ledger_client.cc

@@ -64,7 +64,7 @@ uint64_t MockLedgerClient::LoadURL(const std::string& url,
                  const std::string& contentType,
                  const ledger::URL_METHOD& method,
                  ledger::LedgerCallbackHandler* handler) {
-  handler->OnURLRequestResponse(next_id, 200, "{}")
+  handler->OnURLRequestResponse(next_id, 200, "{}");
   return next_id++;
 }
 

src/url_request_handler.cc

@@ -25,12 +25,15 @@ void URLRequestHandler::OnURLRequestResponse(uint64_t request_id,
   }
 }
 
-bool URLRequestHandler::AddRequestHandler(uint64_t request_id,
-                                          URLRequestCallback callback) {
+bool URLRequestHandler::AddRequestHandler(
+    std::unique_ptr<ledger::LedgerURLLoader> loader,
+    URLRequestCallback callback) {
+  uint64_t request_id = loader->request_id();
   if (request_handlers_.find(request_id) != request_handlers_.end())
     return false;
 
   request_handlers_[request_id] = callback;
+  loader->Start();
   return true;
 }