GonkCameraControl::PullParametersImpl may invalidate cached parameters if the underlying driver modified the values we set for any reason. The following parameters are cached (directly or indirectly) but should not change: CAMERA_PARAM_SUPPORTED_MAXMETERINGAREAS -- mCurrentConfiguration.mMaxMeteringAreas CAMERA_PARAM_SUPPORTED_MAXFOCUSAREAS -- mCurrentConfiguration.mMaxFocusAreas CAMERA_PARAM_SUPPORTED_VIDEOSIZES -- mSeparateVideoAndPreviewSizesSupported CAMERA_PARAM_LUMINANCE -- mLuminanceSupported CAMERA_PARAM_FLASHMODE -- mFlashSupported CAMERA_PARAM_PICTURE_FILEFORMAT -- mFileFormat The followed parameters are cached and may change: CAMERA_PARAM_THUMBNAILSIZE -- mLastThumbnailSize CAMERA_PARAM_PICTURE_SIZE -- mCurrentConfiguration.mPictureSize CAMERA_PARAM_PREVIEWSIZE -- mCurrentConfiguration.mPreviewSize CAMERA_PARAM_VIDEOSIZE -- mLastRecorderSize

Comment on attachment 8602394 [details] [diff] [review] bug1124338, v2 Review of attachment 8602394 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/camera/GonkCameraControl.cpp @@ +1138,5 @@ > + rv = mParams.Get(CAMERA_PARAM_PREVIEWSIZE, mCurrentConfiguration.mPreviewSize); > + } > + if (NS_SUCCEEDED(rv)) { > + rv = mParams.Get(CAMERA_PARAM_VIDEOSIZE, mLastRecorderSize); > + } Indentation should be two spaces. The return values actually should be independent (i.e. while it *shouldn't* the camera could in theory choose to clear the thumbnail size, while still having a valid picture/preview/recorder size) so they shouldn't chain like this. I'd say just ignore them. Otherwise this looks fine. Now just add the test case :).

Comment on attachment 8607920 [details] [diff] [review] bug1124338, v3 Review of attachment 8607920 [details] [diff] [review]: ----------------------------------------------------------------- Almost there :). ::: dom/camera/test/test_camera_fake_parameters.html @@ +532,5 @@ > + var sync = new Promise(function(resolve, reject) { > + function onEvent(e) { > + suite.camera.removeEventListener('focus', onEvent); > + ok(e.newState === 'focused', 'autofocus event state focusing == ' + e.newState); > + ok(suite.hw.params['preview-size'] === '640x480', 'preview size reset with auto focus'); What is stored in suite.hw.params is the simulated camera hardware. What you want to check here is that the Gecko code got configured correctly. You should check that via the DOM API. Preview size is a bit tricky, I'd recommend using the thumbnail size: https://dxr.mozilla.org/mozilla-central/source/dom/webidl/CameraControl.webidl#277 The thumbnail parameter names are here: https://dxr.mozilla.org/mozilla-central/source/dom/camera/FallbackCameraPlatform.cpp#36 You should also configure a default thumbnail size which is different from the new one you set in triggerAutoFocus, so you can verify the original value, and the new value when it should have changed. @@ +539,5 @@ > + suite.camera.addEventListener('focus', onEvent); > + }); > + > + suite.hw.params['preview-size'] = '640x480'; > + suite.hw.fireAutoFocusMoving(false); This works because of a timeout in nsGonkCameraControl::OnAutoFocusMoving which triggers nsGonkCameraControl::OnAutoFocusComplete. I would just change it to do OnAutoFocusComplete instead, safer and faster. @@ +542,5 @@ > + suite.hw.params['preview-size'] = '640x480'; > + suite.hw.fireAutoFocusMoving(false); > + return sync; > + } > + Remove whitespace (the line is fine, just remove the spaces on the line).

Comment on attachment 8609891 [details] [diff] [review] bug1124338, v4 Review of attachment 8609891 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2934d487bd38