|
|
Created:
4 years, 3 months ago by Avi (use Gerrit) Modified:
4 years, 3 months ago CC:
chromium-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove a use of stl_util in safe browsing.
BUG=555865
Committed: https://crrev.com/192c586f9e51cfa5f46137515375966bd72c7899
Cr-Commit-Position: refs/heads/master@{#415108}
Patch Set 1 #
Total comments: 6
Patch Set 2 : cleaner #
Messages
Total messages: 24 (15 generated)
The CQ bit was checked by avi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
avi@chromium.org changed reviewers: + mattm@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by avi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... File components/safe_browsing_db/v4_get_hash_protocol_manager.cc (right): https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... components/safe_browsing_db/v4_get_hash_protocol_manager.cc:96: FullHashCallback callback) const ref https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... File components/safe_browsing_db/v4_get_hash_protocol_manager.h (right): https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... components/safe_browsing_db/v4_get_hash_protocol_manager.h:143: std::unique_ptr<FetcherAndCallback>> Is the unique_ptr here so FetcherAndCallback can be forward declared? What do you think about just using std::pair instead of the FetcherAndCallback struct?
vabr@chromium.org changed reviewers: + vabr@chromium.org
https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... File components/safe_browsing_db/v4_get_hash_protocol_manager.h (right): https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... components/safe_browsing_db/v4_get_hash_protocol_manager.h:142: typedef base::hash_map<const net::URLFetcher*, drive-by nit: https://chromium-cpp.appspot.com/ prefers 'using new_alias = typename' to 'typedef typename new_alias'.
The CQ bit was checked by avi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... File components/safe_browsing_db/v4_get_hash_protocol_manager.cc (right): https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... components/safe_browsing_db/v4_get_hash_protocol_manager.cc:96: FullHashCallback callback) On 2016/08/27 01:43:25, mattm wrote: > const ref moot https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... File components/safe_browsing_db/v4_get_hash_protocol_manager.h (right): https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... components/safe_browsing_db/v4_get_hash_protocol_manager.h:142: typedef base::hash_map<const net::URLFetcher*, On 2016/08/29 09:14:50, vabr (Chromium) wrote: > drive-by nit: > https://chromium-cpp.appspot.com/ prefers 'using new_alias = typename' to > 'typedef typename new_alias'. Done. https://codereview.chromium.org/2284863002/diff/1/components/safe_browsing_db... components/safe_browsing_db/v4_get_hash_protocol_manager.h:143: std::unique_ptr<FetcherAndCallback>> On 2016/08/27 01:43:25, mattm wrote: > Is the unique_ptr here so FetcherAndCallback can be forward declared? I actually don't remember why I did it that specific way. > What do you think about just using std::pair instead of the FetcherAndCallback > struct? Not a huge fan of the whole "first"/"second" thing, but it would probably be simpler.
Not an owner, but LGTM. Vaclav
lgtm
The CQ bit was unchecked by avi@chromium.org
The CQ bit was checked by avi@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Remove a use of stl_util in safe browsing. BUG=555865 ========== to ========== Remove a use of stl_util in safe browsing. BUG=555865 Committed: https://crrev.com/192c586f9e51cfa5f46137515375966bd72c7899 Cr-Commit-Position: refs/heads/master@{#415108} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/192c586f9e51cfa5f46137515375966bd72c7899 Cr-Commit-Position: refs/heads/master@{#415108} |