Implemented argument wrapping for CallJS.

Implemented argument wrapping for CallJS.

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

[Denis Kuznetsov (DE-MUC), 2013-04-17 17:12:19]

https://chromiumcodereview.appspot.com/14063018/diff/1/chrome/browser/ui/webu...
File chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc (right):

https://chromiumcodereview.appspot.com/14063018/diff/1/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc:180:
CallJS("cr.ui.Oobe.setLabelText", UTF8ToUTF16(id), UTF8ToUTF16(text));
Actually, StringValue does UTF16ToUTF8(in_value) conversion inside, and as we
have StringValue auto-conversion from both std::string and string16, we can omit
these conversions here.
Same at line 175.

[bartfab (slow), 2013-04-18 07:39:30]

lgtm with some nits

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc (right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc:8: #include
"base/utf_string_conversions.h"
Nit: No longer used.

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File
chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc
(right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc:432:
l10n_util::GetStringUTF8(message_id));
Nit: Better use GetStringUTF16(). GetStringUTF8() converts UTF16 to UTF8 which
CallJS then implicitly converts back to UTF16.

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc:10: #include
"base/values.h"
Nit: No longer used.

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/eula_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/eula_screen_handler.cc:9: #include
"base/values.h"
Nit: No longer used.

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/terms_of_service_screen_handler.cc
(right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/terms_of_service_screen_handler.cc:7:
#include "base/values.h"
Nit: No longer used.

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
(right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc:239:
CallJS("oobe.UserImageScreen.setCameraPresent", present);
Nit: You can inline the comparison here:

CallJS("oobe.UserImageScreen.setCameraPresent",
       CameraDetector::camera_presence() == CameraDetector::kCameraPresent);

Alternatively, if you prefer having a |present| variable:
1/ Make it const.
2/ Remove the unnecessary braces around "CameraDetector::camera_presence() ==
CameraDetector::kCameraPresent"

[Denis Kuznetsov (DE-MUC), 2013-04-18 07:46:52]

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
(right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc:239:
CallJS("oobe.UserImageScreen.setCameraPresent", present);
On 2013/04/18 07:39:31, bartfab wrote:
> Nit: You can inline the comparison here:
> 
> CallJS("oobe.UserImageScreen.setCameraPresent",
>        CameraDetector::camera_presence() == CameraDetector::kCameraPresent);
> 
> Alternatively, if you prefer having a |present| variable:
> 1/ Make it const.
> 2/ Remove the unnecessary braces around "CameraDetector::camera_presence() ==
> CameraDetector::kCameraPresent"
That would actually be gone when https://codereview.chromium.org/14063017/ lands

[ygorshenin1, 2013-04-18 07:58:23]

Thanks!

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc (right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc:8: #include
"base/utf_string_conversions.h"
On 2013/04/18 07:39:31, bartfab wrote:
> Nit: No longer used.

Done.

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File
chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc
(right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc:432:
l10n_util::GetStringUTF8(message_id));
On 2013/04/18 07:39:31, bartfab wrote:
> Nit: Better use GetStringUTF16(). GetStringUTF8() converts UTF16 to UTF8 which
> CallJS then implicitly converts back to UTF16.

Done.

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc:10: #include
"base/values.h"
On 2013/04/18 07:39:31, bartfab wrote:
> Nit: No longer used.

Done.

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/eula_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/eula_screen_handler.cc:9: #include
"base/values.h"
On 2013/04/18 07:39:31, bartfab wrote:
> Nit: No longer used.

Done.

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/terms_of_service_screen_handler.cc
(right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/terms_of_service_screen_handler.cc:7:
#include "base/values.h"
On 2013/04/18 07:39:31, bartfab wrote:
> Nit: No longer used.

Done.

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
(right):

https://chromiumcodereview.appspot.com/14063018/diff/2002/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc:239:
CallJS("oobe.UserImageScreen.setCameraPresent", present);
On 2013/04/18 07:46:52, Denis Kuznetsov wrote:
> On 2013/04/18 07:39:31, bartfab wrote:
> > Nit: You can inline the comparison here:
> > 
> > CallJS("oobe.UserImageScreen.setCameraPresent",
> >        CameraDetector::camera_presence() == CameraDetector::kCameraPresent);
> > 
> > Alternatively, if you prefer having a |present| variable:
> > 1/ Make it const.
> > 2/ Remove the unnecessary braces around "CameraDetector::camera_presence()
==
> > CameraDetector::kCameraPresent"
> That would actually be gone when https://codereview.chromium.org/14063017/
lands

Done.

[commit-bot: I haz the power, 2013-04-18 07:58:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/14063018/14001

[commit-bot: I haz the power, 2013-04-18 10:04:56]

Change committed as 194870