[ios] Do not JSON encode messages sent from WebView to native code.

ios/web/web_state/ui/crw_web_controller.mm

Index: ios/web/web_state/ui/crw_web_controller.mm
diff --git a/ios/web/web_state/ui/crw_web_controller.mm b/ios/web/web_state/ui/crw_web_controller.mm
index ca23114424b25110cd961183512fc18ee9f53b60..b4660f3baeabc142e7f5a543c673a9bed8ceeb54 100644
--- a/ios/web/web_state/ui/crw_web_controller.mm
+++ b/ios/web/web_state/ui/crw_web_controller.mm
@@ -19,7 +19,6 @@
 #include "base/ios/ios_util.h"
 #import "base/ios/ns_error_util.h"
 #include "base/ios/weak_nsobject.h"
-#include "base/json/json_reader.h"
 #include "base/json/string_escape.h"
 #include "base/logging.h"
 #include "base/mac/bind_objc_block.h"
@@ -2705,18 +2704,10 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
     return NO;
   }
 
-  int errorCode = 0;
-  std::string errorMessage;
-  std::unique_ptr<base::Value> inputJSONData(
-      base::JSONReader::ReadAndReturnError(
-          base::SysNSStringToUTF8(scriptMessage.body), false, &errorCode,
-          &errorMessage));
-  if (errorCode) {
-    DLOG(WARNING) << "JSON parse error: %s" << errorMessage.c_str();
-    return NO;
-  }
+  std::unique_ptr<base::Value> messageAsValue =
+      web::ValueResultFromWKResult(scriptMessage.body);
   base::DictionaryValue* message = nullptr;
-  if (!inputJSONData->GetAsDictionary(&message)) {
+  if (!messageAsValue || !messageAsValue->GetAsDictionary(&message)) {
     return NO;
   }
   std::string windowID;
@@ -2928,8 +2919,12 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
     inputMissing = true;
   }
 
-  if (!message->GetInteger("keyCode", &keyCode) || keyCode < 0)
+  double keyCodeAsDouble = 0;
+  if (!message->GetDouble("keyCode", &keyCodeAsDouble) || keyCodeAsDouble < 0) {
     keyCode = web::WebStateObserver::kInvalidFormKeyCode;
+  } else {
+    keyCode = static_cast<double>(keyCode);

[sdefresne, 2016/12/21 13:05:38]

I think this should instead be:

    keyCode = static_cast<int>(keyCodeAsDouble);

And then I'm surprised that no test caught this error as keyCode is initialized
to web::WebStateObserver::kInvalidFormKeyCode. Can we add a test that fails when
this code is incorrect (or should we instead remove keyCode?).

[Eugene But (OOO till 7-30), 2016/12/21 15:58:46]

Good catch. I don't think that casting to |int| will change anything here in
terms of program correctness, so I can't think about test with would catch this
issue.

If I understand casting correctly then assigning double to int does implicit
cast anyways. Am I missing something?


Removing |keyCode| will not be possible without code duplication like this:
if (!message->GetDouble("keyCode", &keyCodeAsDouble) || keyCodeAsDouble < 0) {
  _webStateImpl->OnFormActivityRegistered(formName, fieldName, type, value,
web::WebStateObserver::kInvalidFormKeyCode, inputMissing);
} else {
  _webStateImpl->OnFormActivityRegistered(formName, fieldName, type, value,
static_cast<int>(keyCode), inputMissing);
}
And I'm not sure if that's a better way :)
}

+  }
   _webStateImpl->OnFormActivityRegistered(formName, fieldName, type, value,
                                           keyCode, inputMissing);
   return YES;
@@ -2937,8 +2932,8 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
 
 - (BOOL)handleCredentialsRequestedMessage:(base::DictionaryValue*)message
                                   context:(NSDictionary*)context {
-  int request_id = -1;
-  if (!message->GetInteger("requestId", &request_id)) {
+  double request_id = -1;
+  if (!message->GetDouble("requestId", &request_id)) {
     DLOG(WARNING) << "JS message parameter not found: requestId";
     return NO;
   }
@@ -2963,15 +2958,16 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
   }
   DCHECK(context[web::kUserIsInteractingKey]);
   _webStateImpl->OnCredentialsRequested(
-      request_id, net::GURLWithNSURL(context[web::kOriginURLKey]), unmediated,
-      federations, [context[web::kUserIsInteractingKey] boolValue]);
+      static_cast<int>(request_id),
+      net::GURLWithNSURL(context[web::kOriginURLKey]), unmediated, federations,
+      [context[web::kUserIsInteractingKey] boolValue]);
   return YES;
 }
 
 - (BOOL)handleSignedInMessage:(base::DictionaryValue*)message
                       context:(NSDictionary*)context {
-  int request_id = -1;
-  if (!message->GetInteger("requestId", &request_id)) {
+  double request_id = -1;
+  if (!message->GetDouble("requestId", &request_id)) {
     DLOG(WARNING) << "JS message parameter not found: requestId";
     return NO;
   }
@@ -2982,11 +2978,11 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
       DLOG(WARNING) << "JS message parameter 'credential' is invalid";
       return NO;
     }
-    _webStateImpl->OnSignedIn(request_id,
+    _webStateImpl->OnSignedIn(static_cast<int>(request_id),
                               net::GURLWithNSURL(context[web::kOriginURLKey]),
                               credential);
   } else {
-    _webStateImpl->OnSignedIn(request_id,
+    _webStateImpl->OnSignedIn(static_cast<int>(request_id),
                               net::GURLWithNSURL(context[web::kOriginURLKey]));
   }
   return YES;
@@ -2994,20 +2990,20 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
 
 - (BOOL)handleSignedOutMessage:(base::DictionaryValue*)message
                        context:(NSDictionary*)context {
-  int request_id = -1;
-  if (!message->GetInteger("requestId", &request_id)) {
+  double request_id = -1;
+  if (!message->GetDouble("requestId", &request_id)) {
     DLOG(WARNING) << "JS message parameter not found: requestId";
     return NO;
   }
-  _webStateImpl->OnSignedOut(request_id,
+  _webStateImpl->OnSignedOut(static_cast<int>(request_id),
                              net::GURLWithNSURL(context[web::kOriginURLKey]));
   return YES;
 }
 
 - (BOOL)handleSignInFailedMessage:(base::DictionaryValue*)message
                           context:(NSDictionary*)context {
-  int request_id = -1;
-  if (!message->GetInteger("requestId", &request_id)) {
+  double request_id = -1;
+  if (!message->GetDouble("requestId", &request_id)) {
     DLOG(WARNING) << "JS message parameter not found: requestId";
     return NO;
   }
@@ -3019,11 +3015,12 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
       return NO;
     }
     _webStateImpl->OnSignInFailed(
-        request_id, net::GURLWithNSURL(context[web::kOriginURLKey]),
-        credential);
+        static_cast<int>(request_id),
+        net::GURLWithNSURL(context[web::kOriginURLKey]), credential);
   } else {
     _webStateImpl->OnSignInFailed(
-        request_id, net::GURLWithNSURL(context[web::kOriginURLKey]));
+        static_cast<int>(request_id),
+        net::GURLWithNSURL(context[web::kOriginURLKey]));
   }
   return YES;
 }
@@ -3084,9 +3081,9 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
 
 - (BOOL)handleWindowHistoryGoMessage:(base::DictionaryValue*)message
                              context:(NSDictionary*)context {
-  int delta = 0;
-  if (message->GetInteger("value", &delta)) {
-    [self goDelta:delta];
+  double delta = 0;
+  if (message->GetDouble("value", &delta)) {
+    [self goDelta:static_cast<int>(delta)];
     return YES;
   }
   return NO;