content/test/data/media/face_detection_test.html - Issue 2419273002: Shape Detection: Add two content browser tests for face detection

Side by Side Diff: content/test/data/media/face_detection_test.html

Issue 2419273002: Shape Detection: Add two content browser tests for face detection (Closed)

Patch Set: reillyg@ comments Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <html>

	3 <head>

	4 </head>

	5

	6 <body>

	7 <img id="myImage">

	8 </body>

	9

	10 <script>

	11 var img = document.getElementById("myImage");

	12

	13 function detectFacesOnImageUrl(url) {
	mcasas 2016/10/18 23:15:14 This code, and the content_browsertest, seem to be This code, and the content_browsertest, seem to be ready to detect several faces, but that functionality is not exercised, and I believe we don't expect to do it anytime soon, so consider restricting the code here to support one face or none (including renaming where necessary). xianglu 2016/10/19 23:26:10 Face detection implementation in Java will return Show quoted text On 2016/10/18 23:15:14, mcasas wrote: > This code, and the content_browsertest, seem to be > ready to detect several faces, but that functionality > is not exercised, and I believe we don't expect to do > it anytime soon, so consider restricting the code here > to support one face or none (including renaming where > necessary). Face detection implementation in Java will return a list even if only one face is detected. I think if there are suitable images, test cases for more than one faces should be included in the future as well. How about adding a comment here instead of restricting the code itself? mcasas 2016/10/19 23:41:11 Hmm I guess in that case, no comment is needed, si Show quoted text On 2016/10/19 23:26:10, xianglu wrote: > On 2016/10/18 23:15:14, mcasas wrote: > > This code, and the content_browsertest, seem to be > > ready to detect several faces, but that functionality > > is not exercised, and I believe we don't expect to do > > it anytime soon, so consider restricting the code here > > to support one face or none (including renaming where > > necessary). > > Face detection implementation in Java will return a list even if only one face > is detected. I think if there are suitable images, test cases for more than one > faces should be included in the future as well. How about adding a comment here > instead of restricting the code itself? Hmm I guess in that case, no comment is needed, since a method should not restrict its callers.
	14 img.src = url;

	15 img.onload = function() {

	16 var detector = new FaceDetector();

	17 var results = "";

	18 detector.detect(img)

	19 .then(faceDetectionResult => {
	mcasas 2016/10/18 23:15:13 micro-nit: s/faceDetectionResult/detectedFaces/ ? micro-nit: s/faceDetectionResult/detectedFaces/ ? xianglu 2016/10/19 23:26:10 Done. Show quoted text On 2016/10/18 23:15:13, mcasas wrote: > micro-nit: s/faceDetectionResult/detectedFaces/ ? Done.
	20 for (var i=0; i<faceDetectionResult.length; i++) {

	21 var boundingBox = faceDetectionResult[i];
	mcasas 2016/10/18 23:15:14 const? const? xianglu 2016/10/19 23:26:10 If I understand correctly, const in javascript mea Show quoted text On 2016/10/18 23:15:14, mcasas wrote: > const? If I understand correctly, const in javascript means the variable name is dedicated to a specific object, and it should never be reassigned? Changing content of the referred object is still allowed though.
	22 var result = "x=" + boundingBox.x + ",y=" + boundingBox.y +

	23 ",w=" + boundingBox.width + ",h=" + boundingBox.height;

	24 results += result + "#";

	25 }

	26 window.domAutomationController.send(results);

	27 })

	28 .catch(error => {

	29 var error = new Error("Error during detection:" + error);

	30 window.domAutomationController.send(error.stack);

	31 });

	32
	mcasas 2016/10/18 23:15:14 Remove this empty line? Remove this empty line? xianglu 2016/10/19 23:26:10 Done. Show quoted text On 2016/10/18 23:15:14, mcasas wrote: > Remove this empty line? Done.
	33 }

	34 }

	35 </script>

	36 </html>

OLD	NEW

« content/browser/shapedetection/shapedetection_browsertest.cc ('K') | « content/test/BUILD.gn ('k') | content/test/data/single_face.jpg » ('j') | no next file with comments »