Use distributed notifications instead of polling to determine success/failure of the preference pan…

Use distributed notifications instead of polling to determine success/failure of the preference pane.

BUG=128115
TEST=Manual


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=137292

[Jamie, 2012-05-15 00:33:49]

ptal

[Lambros, 2012-05-15 18:55:34]

lgtm, but please address comment about replacing DCHECK with LOG(ERROR)

http://codereview.chromium.org/10386128/diff/3001/remoting/host/me2me_prefere...
File remoting/host/me2me_preference_pane.mm (right):

http://codereview.chromium.org/10386128/diff/3001/remoting/host/me2me_prefere...
remoting/host/me2me_preference_pane.mm:416: [[NSString alloc]
initWithUTF8String:kUpdateCompleteNotificationName];
nit: stringWithUTF8String might be easier (and doesn't require releasing the
string, I think).

http://codereview.chromium.org/10386128/diff/3001/remoting/host/plugin/daemon...
File remoting/host/plugin/daemon_controller_mac.cc (right):

http://codereview.chromium.org/10386128/diff/3001/remoting/host/plugin/daemon...
remoting/host/plugin/daemon_controller_mac.cc:308:
DCHECK(current_callback_.is_null());
DCHECK seems too strong.  Maybe the user could hit F5 while the pref-pane is
shown, and go through the same flow again?  LOG(ERROR) feels safer.

http://codereview.chromium.org/10386128/diff/3001/remoting/host/plugin/daemon...
remoting/host/plugin/daemon_controller_mac.cc:338:
current_callback_.Run(result);
DCHECK current_callback_ ?

[Jamie, 2012-05-15 20:53:49]

I've addressed most of your comments and also changed the implementation to use
two messages for success and failure rather than one message with a parameter
(which won't work if the preferences pane is ever sandboxed). PTAL

https://chromiumcodereview.appspot.com/10386128/diff/3001/remoting/host/me2me...
File remoting/host/me2me_preference_pane.mm (right):

https://chromiumcodereview.appspot.com/10386128/diff/3001/remoting/host/me2me...
remoting/host/me2me_preference_pane.mm:416: [[NSString alloc]
initWithUTF8String:kUpdateCompleteNotificationName];
On 2012/05/15 18:55:35, Lambros wrote:
> nit: stringWithUTF8String might be easier (and doesn't require releasing the
> string, I think).

Thanks, I wasn't aware of that.

https://chromiumcodereview.appspot.com/10386128/diff/3001/remoting/host/plugi...
File remoting/host/plugin/daemon_controller_mac.cc (right):

https://chromiumcodereview.appspot.com/10386128/diff/3001/remoting/host/plugi...
remoting/host/plugin/daemon_controller_mac.cc:308:
DCHECK(current_callback_.is_null());
On 2012/05/15 18:55:35, Lambros wrote:
> DCHECK seems too strong.  Maybe the user could hit F5 while the pref-pane is
> shown, and go through the same flow again?  LOG(ERROR) feels safer.

I think DCHECK is correct. If the user refreshes the web-page then the old
plugin is destroyed before a new one is created, so |current_callback_| should
always be null when we get here.

https://chromiumcodereview.appspot.com/10386128/diff/3001/remoting/host/plugi...
remoting/host/plugin/daemon_controller_mac.cc:338:
current_callback_.Run(result);
On 2012/05/15 18:55:35, Lambros wrote:
> DCHECK current_callback_ ?

Done.

[Lambros, 2012-05-15 21:22:37]

lgtm

[commit-bot: I haz the power, 2012-05-15 21:28:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/10386128/11001

[commit-bot: I haz the power, 2012-05-15 23:10:35]

Change committed as 137292