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

Issue 6036001: Move directory code from chrome/service to chrome/browser. (Closed)

Created:
10 years ago by Sergey Ulanov
Modified:
9 years, 7 months ago
Reviewers:
dmac, Alpha Left Google
CC:
chromium-reviews, cbentzel+watch_chromium.org, Alpha Left Google, Sergey Ulanov, darin-cc_chromium.org, awong, garykac, Paweł Hajdan Jr., dmac
Visibility:
Public.

Description

Move directory code from chrome/service to chrome/browser. Renamed RemotingDirectoryService to DirectoryAddRequest. It will be used in RemotingSetupFlow to register host. Also added unittests for this code. BUG=67218 TEST=Unittests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=70082

Patch Set 1 #

Total comments: 10

Patch Set 2 : fixed comments #

Patch Set 3 : - #

Patch Set 4 : - #

Patch Set 5 : Fix checkdeps errors #

Patch Set 6 : ERROR_TIMEOUT->ERROR_TIMEOUT_EXPIRED #

Patch Set 7 : merges #

Unified diffs Side-by-side diffs Delta from patch set Stats (+287 lines, -157 lines) Patch
A chrome/browser/remoting/directory_add_request.h View 1 2 3 4 5 1 chunk +75 lines, -0 lines 0 comments Download
A chrome/browser/remoting/directory_add_request.cc View 1 2 3 4 1 chunk +125 lines, -0 lines 0 comments Download
A chrome/browser/remoting/directory_add_request_unittest.cc View 1 2 3 4 1 chunk +82 lines, -0 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/net/http_return.h View 1 chunk +2 lines, -1 line 0 comments Download
D chrome/service/remoting/remoting_directory_service.h View 1 chunk +0 lines, -72 lines 0 comments Download
D chrome/service/remoting/remoting_directory_service.cc View 1 2 3 4 5 6 1 chunk +0 lines, -84 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Sergey Ulanov
10 years ago (2010-12-17 20:31:41 UTC) #1
dmac
I see that you've moved the files and added them to the gyp. Is there ...
10 years ago (2010-12-17 22:00:57 UTC) #2
dmac
Some comments on comments as well... http://codereview.chromium.org/6036001/diff/1/chrome/browser/remoting/directory_add_request.h File chrome/browser/remoting/directory_add_request.h (right): http://codereview.chromium.org/6036001/diff/1/chrome/browser/remoting/directory_add_request.h#newcode18 chrome/browser/remoting/directory_add_request.h:18: // A class ...
10 years ago (2010-12-17 22:05:45 UTC) #3
Sergey Ulanov
> I see that you've moved the files and added them to the > gyp. ...
10 years ago (2010-12-17 22:33:07 UTC) #4
dmac
10 years ago (2010-12-20 21:28:47 UTC) #5
On 2010/12/17 22:33:07, sergeyu wrote:
> > I see that you've moved the files and added them to the
> > gyp. Is there a gyp file
> > that the old files need to be removed from? Did I miss it?
> I removed them in another CL http://codereview.chromium.org/5955001/ (not
> checked in yet).
> 
>
http://codereview.chromium.org/6036001/diff/1/chrome/browser/remoting/directo...
> File chrome/browser/remoting/directory_add_request.h (right):
> 
>
http://codereview.chromium.org/6036001/diff/1/chrome/browser/remoting/directo...
> chrome/browser/remoting/directory_add_request.h:18: // A class implements REST
> API insert call for the Chromoting directory service.
> On 2010/12/17 22:05:46, dmac wrote:
> > // A class that implements the REST API insert call for the Remoting
directory
> > service.
> In all our docs the service is called Chromoting directory.
> 
>
http://codereview.chromium.org/6036001/diff/1/chrome/browser/remoting/directo...
> chrome/browser/remoting/directory_add_request.h:26: // Server rejested request
> because it was invalid. This may
> On 2010/12/17 22:05:46, dmac wrote:
> > Server rejected the
> 
> Done.
> 
>
http://codereview.chromium.org/6036001/diff/1/chrome/browser/remoting/directo...
> chrome/browser/remoting/directory_add_request.h:27: // happen, for example, if
> the specified public key was invalid.
> On 2010/12/17 22:05:46, dmac wrote:
> > get rid of , for example,
> 
> Done.
> 
>
http://codereview.chromium.org/6036001/diff/1/chrome/browser/remoting/directo...
> chrome/browser/remoting/directory_add_request.h:40: // contains error message
in
> case of an error. Error message may not
> On 2010/12/17 22:05:46, dmac wrote:
> > an error message
> > 
> > . The error message
> 
> Done.
> 
>
http://codereview.chromium.org/6036001/diff/1/chrome/browser/remoting/directo...
> chrome/browser/remoting/directory_add_request.h:48: // Add this computer as
> host. Use the token for
> On 2010/12/17 22:05:46, dmac wrote:
> > as a host.
> > 
> > |done_callback| is called when the request is finished.
> > 
> > destroying
> 
> Done.

LGTM

Powered by Google App Engine
This is Rietveld 408576698