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

Issue 1948793002: Implement HostListManager by Refactoring HostListLoader (Closed)

Created:
4 years, 7 months ago by Yuwei
Modified:
4 years, 7 months ago
Reviewers:
Lambros
CC:
chromium-reviews, chromoting-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement HostListManager by Refactoring HostListLoader This CL renamed and refactored HostListLoader into HostListManager and added functionality of updating and deleting a host entry in directory service. BUG=607216 Committed: https://crrev.com/8ee766c67317b0d1c97686c175fa313821cccf61 Cr-Commit-Position: refs/heads/master@{#392083}

Patch Set 1 #

Total comments: 3

Patch Set 2 : Addressed reviewer's feedback #

Patch Set 3 : Make Response clearer #

Total comments: 6

Patch Set 4 : Reviewer's feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+207 lines, -263 lines) Patch
M remoting/android/client_java_tmpl.gni View 1 chunk +1 line, -1 line 0 comments Download
M remoting/android/java/src/org/chromium/chromoting/Chromoting.java View 8 chunks +18 lines, -8 lines 0 comments Download
M remoting/android/java/src/org/chromium/chromoting/HostInfo.java View 1 chunk +1 line, -1 line 0 comments Download
D remoting/android/java/src/org/chromium/chromoting/HostListLoader.java View 1 chunk +0 lines, -194 lines 0 comments Download
A + remoting/android/java/src/org/chromium/chromoting/HostListManager.java View 1 2 3 7 chunks +169 lines, -51 lines 0 comments Download
M remoting/android/java/src/org/chromium/chromoting/SessionConnector.java View 4 chunks +18 lines, -8 lines 0 comments Download

Messages

Total messages: 21 (7 generated)
Yuwei
There will be upcoming CL's for generalizing the OAuthTokenFetcher callback (maybe do it for the ...
4 years, 7 months ago (2016-05-03 21:37:02 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1948793002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1948793002/1
4 years, 7 months ago (2016-05-03 21:41:24 UTC) #4
Lambros
https://codereview.chromium.org/1948793002/diff/1/remoting/android/java/src/org/chromium/chromoting/HostListManager.java File remoting/android/java/src/org/chromium/chromoting/HostListManager.java (right): https://codereview.chromium.org/1948793002/diff/1/remoting/android/java/src/org/chromium/chromoting/HostListManager.java#newcode100 remoting/android/java/src/org/chromium/chromoting/HostListManager.java:100: postError(callback, response.error); If response.error is null, it feels wrong ...
4 years, 7 months ago (2016-05-03 22:11:03 UTC) #5
Yuwei
https://codereview.chromium.org/1948793002/diff/1/remoting/android/java/src/org/chromium/chromoting/HostListManager.java File remoting/android/java/src/org/chromium/chromoting/HostListManager.java (right): https://codereview.chromium.org/1948793002/diff/1/remoting/android/java/src/org/chromium/chromoting/HostListManager.java#newcode100 remoting/android/java/src/org/chromium/chromoting/HostListManager.java:100: postError(callback, response.error); On 2016/05/03 22:11:03, Lambros wrote: > If ...
4 years, 7 months ago (2016-05-03 22:39:02 UTC) #6
Yuwei
https://codereview.chromium.org/1948793002/diff/1/remoting/android/java/src/org/chromium/chromoting/HostListManager.java File remoting/android/java/src/org/chromium/chromoting/HostListManager.java (right): https://codereview.chromium.org/1948793002/diff/1/remoting/android/java/src/org/chromium/chromoting/HostListManager.java#newcode100 remoting/android/java/src/org/chromium/chromoting/HostListManager.java:100: postError(callback, response.error); On 2016/05/03 22:39:02, yuweih wrote: > On ...
4 years, 7 months ago (2016-05-03 22:47:57 UTC) #7
Yuwei
Patch 3 is uploaded. PTAL
4 years, 7 months ago (2016-05-04 22:19:47 UTC) #8
Lambros
lgtm https://codereview.chromium.org/1948793002/diff/40001/remoting/android/java/src/org/chromium/chromoting/HostListManager.java File remoting/android/java/src/org/chromium/chromoting/HostListManager.java (right): https://codereview.chromium.org/1948793002/diff/40001/remoting/android/java/src/org/chromium/chromoting/HostListManager.java#newcode47 remoting/android/java/src/org/chromium/chromoting/HostListManager.java:47: * |error| and |body| can be null and ...
4 years, 7 months ago (2016-05-05 00:00:14 UTC) #9
Yuwei
https://codereview.chromium.org/1948793002/diff/40001/remoting/android/java/src/org/chromium/chromoting/HostListManager.java File remoting/android/java/src/org/chromium/chromoting/HostListManager.java (right): https://codereview.chromium.org/1948793002/diff/40001/remoting/android/java/src/org/chromium/chromoting/HostListManager.java#newcode47 remoting/android/java/src/org/chromium/chromoting/HostListManager.java:47: * |error| and |body| can be null and they ...
4 years, 7 months ago (2016-05-05 00:40:59 UTC) #10
Yuwei
https://codereview.chromium.org/1948793002/diff/40001/remoting/android/java/src/org/chromium/chromoting/HostListManager.java File remoting/android/java/src/org/chromium/chromoting/HostListManager.java (right): https://codereview.chromium.org/1948793002/diff/40001/remoting/android/java/src/org/chromium/chromoting/HostListManager.java#newcode47 remoting/android/java/src/org/chromium/chromoting/HostListManager.java:47: * |error| and |body| can be null and they ...
4 years, 7 months ago (2016-05-06 00:31:58 UTC) #12
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1948793002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1948793002/60001
4 years, 7 months ago (2016-05-06 00:32:15 UTC) #13
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 7 months ago (2016-05-06 01:26:00 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1948793002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1948793002/60001
4 years, 7 months ago (2016-05-06 17:06:16 UTC) #18
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years, 7 months ago (2016-05-06 17:14:17 UTC) #19
commit-bot: I haz the power
4 years, 7 months ago (2016-05-06 17:16:13 UTC) #21
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/8ee766c67317b0d1c97686c175fa313821cccf61
Cr-Commit-Position: refs/heads/master@{#392083}

Powered by Google App Engine
This is Rietveld 408576698