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

Issue 11270039: Add NetworkChangeNotifier connectivity events to the NetLog. (Closed)

Created:
8 years, 1 month ago by pauljensen
Modified:
8 years, 1 month ago
Reviewers:
jam, mmenke
CC:
chromium-reviews, cbentzel+watch_chromium.org, eroman, darin-cc_chromium.org, mmenke
Visibility:
Public.

Description

Add NetworkChangeNotifier connectivity events to the NetLog. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=164531

Patch Set 1 #

Total comments: 5

Patch Set 2 : Address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+53 lines, -4 lines) Patch
M chrome/browser/io_thread.cc View 1 1 chunk +18 lines, -2 lines 0 comments Download
M net/base/net_log_event_type_list.h View 1 1 chunk +9 lines, -2 lines 0 comments Download
M net/base/network_change_notifier.h View 1 chunk +3 lines, -0 lines 0 comments Download
M net/base/network_change_notifier.cc View 1 1 chunk +23 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
pauljensen
Matt, could you review this little change? It looks like when LoggingNetworkChangeObserver was added the ...
8 years, 1 month ago (2012-10-25 16:14:45 UTC) #1
mmenke
http://codereview.chromium.org/11270039/diff/1/chrome/browser/io_thread.cc File chrome/browser/io_thread.cc (right): http://codereview.chromium.org/11270039/diff/1/chrome/browser/io_thread.cc#newcode268 chrome/browser/io_thread.cc:268: virtual void OnConnectionTypeChanged( While you're here, could you make ...
8 years, 1 month ago (2012-10-25 16:25:31 UTC) #2
pauljensen
Thanks for the comments! I believe I have addressed them all. http://codereview.chromium.org/11270039/diff/1/net/base/network_change_notifier.cc File net/base/network_change_notifier.cc (right): ...
8 years, 1 month ago (2012-10-25 17:08:14 UTC) #3
mmenke
LGTM! And thanks for adding this - sure it'll come in handy in the future. ...
8 years, 1 month ago (2012-10-25 17:34:57 UTC) #4
pauljensen
Scott, could you approve chrome/browser/io_thread.cc? Matt has already reviewed the change as a whole.
8 years, 1 month ago (2012-10-25 21:34:54 UTC) #5
sky
I'm not a good reviewer for io_thread. Maybe John.
8 years, 1 month ago (2012-10-25 21:57:09 UTC) #6
jam
On 2012/10/25 21:57:09, sky wrote: > I'm not a good reviewer for io_thread. Maybe John. ...
8 years, 1 month ago (2012-10-26 00:47:13 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pauljensen@chromium.org/11270039/8001
8 years, 1 month ago (2012-10-26 13:38:00 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pauljensen@chromium.org/11270039/8001
8 years, 1 month ago (2012-10-27 12:22:40 UTC) #9
commit-bot: I haz the power
8 years, 1 month ago (2012-10-27 12:31:29 UTC) #10
Change committed as 164531

Powered by Google App Engine
This is Rietveld 408576698