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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 318283002: Added new GCD/Privet interfaces. (Closed)

Created:
6 years, 6 months ago by Vitaly Buka (NO REVIEWS)
Modified:
6 years, 6 months ago
Reviewers:
Noam Samuel
CC:
chromium-reviews
Visibility:
Public.

Description

Added GCDApiFlowInterface for simpler testing. Added empty PrivetV3HTTPClient and implementation. Reordered LocalDiscoveryUIHandler member to match dependency. BUG=372843 TBR=noamsml Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=275742

Patch Set 1 #

Patch Set 2 : #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+171 lines, -96 lines) Patch
M chrome/browser/local_discovery/cloud_device_list.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/local_discovery/cloud_device_list.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/local_discovery/cloud_print_printer_list.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/local_discovery/cloud_print_printer_list.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/local_discovery/gcd_api_flow.h View 4 chunks +19 lines, -7 lines 1 comment Download
M chrome/browser/local_discovery/gcd_api_flow.cc View 1 chunk +15 lines, -10 lines 0 comments Download
M chrome/browser/local_discovery/gcd_api_flow_unittest.cc View 1 chunk +3 lines, -5 lines 0 comments Download
M chrome/browser/local_discovery/gcd_registration_ticket_request.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/local_discovery/gcd_registration_ticket_request.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/local_discovery/privet_confirm_api_flow.h View 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/local_discovery/privet_confirm_api_flow.cc View 2 chunks +5 lines, -4 lines 0 comments Download
M chrome/browser/local_discovery/privet_confirm_api_flow_unittest.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M chrome/browser/local_discovery/privet_http.h View 2 chunks +19 lines, -3 lines 0 comments Download
M chrome/browser/local_discovery/privet_http.cc View 1 chunk +11 lines, -0 lines 0 comments Download
M chrome/browser/local_discovery/privet_http_impl.h View 1 chunk +17 lines, -0 lines 0 comments Download
M chrome/browser/local_discovery/privet_http_impl.cc View 1 chunk +17 lines, -0 lines 0 comments Download
M chrome/browser/local_discovery/privetv3_session.h View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/local_discovery/privetv3_session.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/local_discovery/privetv3_setup_flow.h View 3 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/ui/webui/local_discovery/local_discovery_ui_handler.h View 7 chunks +17 lines, -23 lines 0 comments Download
M chrome/browser/ui/webui/local_discovery/local_discovery_ui_handler.cc View 1 4 chunks +24 lines, -23 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Vitaly Buka (NO REVIEWS)
The CQ bit was checked by vitalybuka@chromium.org
6 years, 6 months ago (2014-06-08 06:03:18 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/318283002/1
6 years, 6 months ago (2014-06-08 06:03:53 UTC) #2
Vitaly Buka (NO REVIEWS)
The CQ bit was checked by vitalybuka@chromium.org
6 years, 6 months ago (2014-06-08 07:22:04 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/318283002/40001
6 years, 6 months ago (2014-06-08 07:22:12 UTC) #4
commit-bot: I haz the power
Change committed as 275742
6 years, 6 months ago (2014-06-08 11:25:44 UTC) #5
Noam Samuel
lgtm https://codereview.chromium.org/318283002/diff/40001/chrome/browser/local_discovery/gcd_api_flow.h File chrome/browser/local_discovery/gcd_api_flow.h (right): https://codereview.chromium.org/318283002/diff/40001/chrome/browser/local_discovery/gcd_api_flow.h#newcode20 chrome/browser/local_discovery/gcd_api_flow.h:20: class GCDApiFlowInterface { Isn't it more common to ...
6 years, 6 months ago (2014-06-09 17:53:22 UTC) #6
Vitaly Buka (NO REVIEWS)
6 years, 6 months ago (2014-06-09 21:27:46 UTC) #7
Message was sent while issue was closed. 
On 2014/06/09 17:53:22, Noam Samuel wrote:
> lgtm
> 
>
https://codereview.chromium.org/318283002/diff/40001/chrome/browser/local_dis...
> File chrome/browser/local_discovery/gcd_api_flow.h (right):
> 
>
https://codereview.chromium.org/318283002/diff/40001/chrome/browser/local_dis...
> chrome/browser/local_discovery/gcd_api_flow.h:20: class GCDApiFlowInterface {
> Isn't it more common to write GCDApiFlow and GCDApiFlowImpl?

ACK

We can do this. Still I don't like to have *Impl in public interface. So maybe
some static Create would be needed.

Powered by Google App Engine
This is Rietveld 408576698