Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(287)

Issue 2430953004: Merge ProxyResolverMojo::Job and ProxyResolverMojo::RequestImpl (Closed)

Created:
4 years, 2 months ago by maksims (do not use this acc)
Modified:
4 years, 2 months ago
Reviewers:
Sam McNally, eroman
CC:
chromium-reviews, cbentzel+watch_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review)
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Merge ProxyResolverMojo::Job and ProxyResolverMojo::RequestImpl This is a follow up CL, which merges RequestImpl and Job together. Prev CL is here - https://chromiumcodereview.appspot.com/2299963002/ BUG=478934 Committed: https://crrev.com/f1aa7d4e5ec785cc01875b4eecf46844161d6d3c Cr-Commit-Position: refs/heads/master@{#426862}

Patch Set 1 #

Total comments: 1

Patch Set 2 : nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -26 lines) Patch
M net/proxy/proxy_resolver_factory_mojo.cc View 1 5 chunks +4 lines, -26 lines 0 comments Download

Messages

Total messages: 19 (14 generated)
maksims (do not use this acc)
as per our discussion, here is the follow up.
4 years, 2 months ago (2016-10-20 13:33:15 UTC) #5
eroman
lgtm https://codereview.chromium.org/2430953004/diff/1/net/proxy/proxy_resolver_factory_mojo.cc File net/proxy/proxy_resolver_factory_mojo.cc (right): https://codereview.chromium.org/2430953004/diff/1/net/proxy/proxy_resolver_factory_mojo.cc#newcode277 net/proxy/proxy_resolver_factory_mojo.cc:277: *request = std::move(job); How about combining these two ...
4 years, 2 months ago (2016-10-20 16:53:37 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2430953004/20001
4 years, 2 months ago (2016-10-21 18:54:38 UTC) #15
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 2 months ago (2016-10-21 19:25:31 UTC) #17
commit-bot: I haz the power
4 years, 2 months ago (2016-10-21 19:44:57 UTC) #19
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/f1aa7d4e5ec785cc01875b4eecf46844161d6d3c
Cr-Commit-Position: refs/heads/master@{#426862}

Powered by Google App Engine
This is Rietveld 408576698