Added confirmation for printer registration.

Added confirmation for printer registration.

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213498

[maksymb, 2013-07-17 01:25:27]

Please take a look at my CL.

[maksymb, 2013-07-17 01:28:19]

https://codereview.chromium.org/19468002/diff/2001/cloud_print/gcp20/prototyp...
File cloud_print/gcp20/prototype/printer.cc (right):

https://codereview.chromium.org/19468002/diff/2001/cloud_print/gcp20/prototyp...
cloud_print/gcp20/prototype/printer.cc:100: #if !defined(OS_WIN)
I don't like to see all this stuff here, in this file. Is it better to move it
to separate file?

[Vitaly Buka (NO REVIEWS), 2013-07-17 06:59:00]

https://codereview.chromium.org/19468002/diff/2001/cloud_print/gcp20/prototyp...
File cloud_print/gcp20/prototype/printer.cc (right):

https://codereview.chromium.org/19468002/diff/2001/cloud_print/gcp20/prototyp...
cloud_print/gcp20/prototype/printer.cc:36: const char* kServiceNamePrefix =
"first_gcp20_device";
Just noticed, please replace all such consts with 
const char kServiceType[] = "_privet._tcp.local";

https://codereview.chromium.org/19468002/diff/2001/cloud_print/gcp20/prototyp...
cloud_print/gcp20/prototype/printer.cc:100: #if !defined(OS_WIN)
On 2013/07/17 01:28:20, maksymb wrote:
> I don't like to see all this stuff here, in this file. Is it better to move it
> to separate file?

sure, you can put this in some file with _posix.cc suffix

[maksymb, 2013-07-17 18:24:50]

https://codereview.chromium.org/19468002/diff/2001/cloud_print/gcp20/prototyp...
File cloud_print/gcp20/prototype/printer.cc (right):

https://codereview.chromium.org/19468002/diff/2001/cloud_print/gcp20/prototyp...
cloud_print/gcp20/prototype/printer.cc:36: const char* kServiceNamePrefix =
"first_gcp20_device";
On 2013/07/17 06:59:00, Vitaly Buka wrote:
> Just noticed, please replace all such consts with 
> const char kServiceType[] = "_privet._tcp.local";

Done.

https://codereview.chromium.org/19468002/diff/2001/cloud_print/gcp20/prototyp...
cloud_print/gcp20/prototype/printer.cc:100: #if !defined(OS_WIN)
On 2013/07/17 06:59:00, Vitaly Buka wrote:
> On 2013/07/17 01:28:20, maksymb wrote:
> > I don't like to see all this stuff here, in this file. Is it better to move
it
> > to separate file?
> 
> sure, you can put this in some file with _posix.cc suffix

Done.

[Vitaly Buka (NO REVIEWS), 2013-07-17 18:35:56]

lgtm

[gene, 2013-07-18 09:56:46]

I have a few questions below. May be I am just reading code wrong.

https://codereview.chromium.org/19468002/diff/11002/cloud_print/gcp20/prototy...
File cloud_print/gcp20/prototype/printer.cc (right):

https://codereview.chromium.org/19468002/diff/11002/cloud_print/gcp20/prototy...
cloud_print/gcp20/prototype/printer.cc:175: printf("%s\n",
kUserConfirmationTitle);
Why don't you just printf(kUserConfirmationTitle); here??
You can add '\n' to the const.

https://codereview.chromium.org/19468002/diff/11002/cloud_print/gcp20/prototy...
cloud_print/gcp20/prototype/printer.cc:207: return
PrivetHttpServer::REG_ERROR_INVALID_ACTION;
I think it should be different errors if registration not started vs token is
not ready 
Actually there should be different errors if user confirmation timeout or user
cancels it on the printer

[Vitaly Buka (NO REVIEWS), 2013-07-22 04:01:23]

https://chromiumcodereview.appspot.com/19468002/diff/33001/cloud_print/gcp20/...
File cloud_print/gcp20/prototype/dns_sd_server.cc (right):

https://chromiumcodereview.appspot.com/19468002/diff/33001/cloud_print/gcp20/...
cloud_print/gcp20/prototype/dns_sd_server.cc:69:
base::Bind(&DnsSdServer::OnDatagramReceived, base::Unretained(this)));
can't use unretained here

https://chromiumcodereview.appspot.com/19468002/diff/33001/cloud_print/gcp20/...
cloud_print/gcp20/prototype/dns_sd_server.cc:288:
base::Bind(&DnsSdServer::Update, base::Unretained(this)),
can't use unretained here

https://chromiumcodereview.appspot.com/19468002/diff/33001/cloud_print/gcp20/...
File cloud_print/gcp20/prototype/printer.cc (right):

https://chromiumcodereview.appspot.com/19468002/diff/33001/cloud_print/gcp20/...
cloud_print/gcp20/prototype/printer.cc:180:
base::Bind(&Printer::WaitUserConfirmation,
can't use unretained here

https://chromiumcodereview.appspot.com/19468002/diff/33001/cloud_print/gcp20/...
cloud_print/gcp20/prototype/printer.cc:353:
base::Bind(&Printer::WaitUserConfirmation, base::Unretained(this),
can't use unretained here

[maksymb, 2013-07-22 19:52:16]

Added some comments for states of registration.

https://codereview.chromium.org/19468002/diff/11002/cloud_print/gcp20/prototy...
File cloud_print/gcp20/prototype/printer.cc (right):

https://codereview.chromium.org/19468002/diff/11002/cloud_print/gcp20/prototy...
cloud_print/gcp20/prototype/printer.cc:175: printf("%s\n",
kUserConfirmationTitle);
On 2013/07/18 09:56:46, gene wrote:
> Why don't you just printf(kUserConfirmationTitle); here??
> You can add '\n' to the const.

Done.

https://codereview.chromium.org/19468002/diff/11002/cloud_print/gcp20/prototy...
cloud_print/gcp20/prototype/printer.cc:207: return
PrivetHttpServer::REG_ERROR_INVALID_ACTION;
On 2013/07/18 09:56:46, gene wrote:
> I think it should be different errors if registration not started vs token is
> not ready 
> Actually there should be different errors if user confirmation timeout or user
> cancels it on the printer

Added comments.

https://codereview.chromium.org/19468002/diff/33001/cloud_print/gcp20/prototy...
File cloud_print/gcp20/prototype/dns_sd_server.cc (right):

https://codereview.chromium.org/19468002/diff/33001/cloud_print/gcp20/prototy...
cloud_print/gcp20/prototype/dns_sd_server.cc:69:
base::Bind(&DnsSdServer::OnDatagramReceived, base::Unretained(this)));
On 2013/07/22 04:01:23, Vitaly Buka wrote:
> can't use unretained here

Done.

https://codereview.chromium.org/19468002/diff/33001/cloud_print/gcp20/prototy...
cloud_print/gcp20/prototype/dns_sd_server.cc:288:
base::Bind(&DnsSdServer::Update, base::Unretained(this)),
On 2013/07/22 04:01:23, Vitaly Buka wrote:
> can't use unretained here

Done.

https://codereview.chromium.org/19468002/diff/33001/cloud_print/gcp20/prototy...
File cloud_print/gcp20/prototype/printer.cc (right):

https://codereview.chromium.org/19468002/diff/33001/cloud_print/gcp20/prototy...
cloud_print/gcp20/prototype/printer.cc:180:
base::Bind(&Printer::WaitUserConfirmation,
On 2013/07/22 04:01:23, Vitaly Buka wrote:
> can't use unretained here

Done.

https://codereview.chromium.org/19468002/diff/33001/cloud_print/gcp20/prototy...
cloud_print/gcp20/prototype/printer.cc:353:
base::Bind(&Printer::WaitUserConfirmation, base::Unretained(this),
On 2013/07/22 04:01:23, Vitaly Buka wrote:
> can't use unretained here

Done.

[gene, 2013-07-23 11:31:35]

lgtm

[commit-bot: I haz the power, 2013-07-23 23:40:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maksymb@chromium.org/19468002/111001

[commit-bot: I haz the power, 2013-07-24 00:42:13]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-07-24 17:06:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maksymb@chromium.org/19468002/126001

[commit-bot: I haz the power, 2013-07-24 20:10:58]

Change committed as 213498