Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(122)

Issue 29408721: Issue 5002 - Fix potential concurrency issues in WebRequest tests. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 10 months ago by hub
Modified:
2 years, 10 months ago
Reviewers:
sergei
Base URL:
https://hg.adblockplus.org/libadblockplus/
Visibility:
Public.

Description

Issue 5002 - Fix potential concurrency issues in WebRequest tests.

Patch Set 1 #

Total comments: 8

Patch Set 2 : Fixes from review. #

Total comments: 4

Patch Set 3 : Avoid casting the webRequest pointer. #

Total comments: 4

Patch Set 4 : Fixed the last nits. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+85 lines, -30 lines) Patch
M test/WebRequest.cpp View 1 2 3 3 chunks +85 lines, -30 lines 0 comments Download

Messages

Total messages: 8
hub
2 years, 10 months ago (2017-04-10 12:41:25 UTC) #1
sergei
https://codereview.adblockplus.org/29408721/diff/29408722/test/WebRequest.cpp File test/WebRequest.cpp (right): https://codereview.adblockplus.org/29408721/diff/29408722/test/WebRequest.cpp#newcode33 test/WebRequest.cpp:33: std::set<std::string> r; what about renaming here? r -> headerNames ...
2 years, 10 months ago (2017-04-19 10:38:31 UTC) #2
hub
updated patch https://codereview.adblockplus.org/29408721/diff/29408722/test/WebRequest.cpp File test/WebRequest.cpp (right): https://codereview.adblockplus.org/29408721/diff/29408722/test/WebRequest.cpp#newcode33 test/WebRequest.cpp:33: std::set<std::string> r; On 2017/04/19 10:38:31, sergei wrote: ...
2 years, 10 months ago (2017-04-19 13:12:05 UTC) #3
sergei
https://codereview.adblockplus.org/29408721/diff/29417568/test/WebRequest.cpp File test/WebRequest.cpp (right): https://codereview.adblockplus.org/29408721/diff/29417568/test/WebRequest.cpp#newcode64 test/WebRequest.cpp:64: const auto& iter = requestHeaderNames.find(url); const reference keeps the ...
2 years, 10 months ago (2017-04-19 13:17:29 UTC) #4
hub
updated patch https://codereview.adblockplus.org/29408721/diff/29417568/test/WebRequest.cpp File test/WebRequest.cpp (right): https://codereview.adblockplus.org/29408721/diff/29417568/test/WebRequest.cpp#newcode64 test/WebRequest.cpp:64: const auto& iter = requestHeaderNames.find(url); On 2017/04/19 ...
2 years, 10 months ago (2017-04-19 14:41:00 UTC) #5
sergei
LGTM https://codereview.adblockplus.org/29408721/diff/29417570/test/WebRequest.cpp File test/WebRequest.cpp (right): https://codereview.adblockplus.org/29408721/diff/29417570/test/WebRequest.cpp#newcode86 test/WebRequest.cpp:86: webRequest = std::shared_ptr<T>(new T()); std::make_shared are usually preferable, ...
2 years, 10 months ago (2017-04-19 14:44:47 UTC) #6
hub
Update the patch to make things right. https://codereview.adblockplus.org/29408721/diff/29417570/test/WebRequest.cpp File test/WebRequest.cpp (right): https://codereview.adblockplus.org/29408721/diff/29417570/test/WebRequest.cpp#newcode86 test/WebRequest.cpp:86: webRequest = ...
2 years, 10 months ago (2017-04-19 14:55:06 UTC) #7
sergei
2 years, 10 months ago (2017-04-19 15:01:55 UTC) #8
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5