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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 8764006: chrome: chromeos: dbus: simplify ProxyResolutionServiceProvider unittest (Closed)

Created:
9 years ago by Vince Laviano
Modified:
9 years ago
CC:
chromium-reviews, stevenjb+watch_chromium.org, nkostylev+watch_chromium.org, davemoore+watch_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

chrome: chromeos: dbus: simplify ProxyResolutionServiceProvider unittest This is a follow-on CL, in response to a code review comment at http://codereview.chromium.org/8728020/ , that simplifies the ProxyResolutionServiceProvider unittest to run its main loop at most once rather than in a loop. BUG=chromium-os:23241 TEST=Run unit test Change-Id: I3e8a730679f430f1966073422626ebffbd1a03bf Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112567

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fix style nit #

Patch Set 3 : Fix whitespace #

Total comments: 2

Patch Set 4 : Fix comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -6 lines) Patch
M chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc View 1 2 3 2 chunks +4 lines, -6 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Paweł Hajdan Jr.
LGTM, thank you. http://codereview.chromium.org/8764006/diff/1/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc File chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc (right): http://codereview.chromium.org/8764006/diff/1/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc#newcode265 chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc:265: if (!response_received_) { nit: Remove the ...
9 years ago (2011-12-01 09:24:47 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vlaviano@chromium.org/8764006/2002
9 years ago (2011-12-01 21:12:28 UTC) #2
commit-bot: I haz the power
Presubmit check for 8764006-2002 failed and returned exit status 1. Running presubmit commit checks ...
9 years ago (2011-12-01 21:12:30 UTC) #3
stevenjb
owner lgtm http://codereview.chromium.org/8764006/diff/2002/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc File chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc (right): http://codereview.chromium.org/8764006/diff/2002/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc#newcode264 chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc:264: // Wait for a response. nit: s/Wait/Check?
9 years ago (2011-12-01 21:43:38 UTC) #4
Vince Laviano
http://codereview.chromium.org/8764006/diff/1/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc File chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc (right): http://codereview.chromium.org/8764006/diff/1/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc#newcode265 chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc:265: if (!response_received_) { On 2011/12/01 09:24:48, Paweł Hajdan Jr. ...
9 years ago (2011-12-01 21:53:30 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vlaviano@chromium.org/8764006/5003
9 years ago (2011-12-01 21:53:51 UTC) #6
commit-bot: I haz the power
9 years ago (2011-12-01 23:05:10 UTC) #7 
Change committed as 112567

Powered by Google App Engine
This is Rietveld 408576698