DesktopCaptureApiTest.ChooseDesktopMedia reports an Uncaught API schema error, but still passes |
|||
Issue description
Chrome Version: 64.0.3268.0 (Developer Debug build)
OS: Linux
What steps will reproduce the problem?
(1) Run Browser_tests DesktopCaptureApiTest.ChooseDesktopMedia
What is the expected result?
Expect that test passes with no errors reported.
Expect that if errors are reported then test fails.
What happens instead?
Test passes, but reports API error:
[33131:33131:1113/113703.024030:INFO:CONSOLE(0)] "(BLESSED_EXTENSION context for knldjmfmopnpolahpmmgbagdohdnhkik) extensions::schemaUtils:36: Uncaught Error: Parameter 2 (options) is required.{Error: Parameter 2 (options) is required.
at validate (extensions::schemaUtils:36:13)
at handleResponse (extensions::sendRequest:65:9)}", source: chrome-extension://knldjmfmopnpolahpmmgbagdohdnhkik/_generated_background_page.html (0)
,
Nov 15 2017
It turns out to be an existing problem in chrome.desktopCapture.cancelChooseDesktopMedia, or in Chrome Extensions, with debug building. Those error logs comes from 'cancelDialog()' case, which will call cancelChooseDesktopMedia() eventually at https://cs.chromium.org/chromium/src/chrome/test/data/extensions/api_test/desktop_capture/test.js?sq=package:chromium&dr&l=122. Within cancelChooseDesktopMedia(), it will call "RespondNow(NoArguments())" as return, in which causes ShemaUtils.validate error at https://cs.chromium.org/chromium/src/extensions/renderer/resources/schema_utils.js?l=36. I'm contacting Extension guys to see what can be done.
,
Nov 16 2017
Thanks to rdcronin@! He helped to figure out the problem is actually in DesktopCaptureChooseDesktopMediaFunctionBase::Cancel(), https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc?l=73. Since the shema has two parameters, https://cs.chromium.org/chromium/src/chrome/common/extensions/api/desktop_capture.json?l=38, so whenever we set the result, those two parameters must be passed in, otherwise shemaUtils.validate() will throw an error. I guess this problem has existed for about one year since the option for audio capture was added. A cl is uploaded, waiting for passing try bots.
,
Nov 16 2017
Great! Do we understand why the test passes in spite of the cancel case not actually working as intended?
,
Nov 16 2017
I guess the js exception won't block the test and we eventually give "chrome.test.succeed();" in cancelDialog() case, https://cs.chromium.org/chromium/src/chrome/test/data/extensions/api_test/desktop_capture/test.js?sq=package:chromium&dr&l=123.
,
Nov 16 2017
Shouldn't the cancelChooseDesktopMedia() call have trigger a JavaScript exception, preventing us from reaching the chrome.test.succeed() line, though? Or are schema errors somehow not reported back to JS?
,
Nov 16 2017
It looks like that I can't catch the thrown error in test.js. No idea about the Extensions implementation at all. Any insight?
,
Nov 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e334368d858f03a8fd653f72eebf37675c16d585 commit e334368d858f03a8fd653f72eebf37675c16d585 Author: Weiyong Yao <braveyao@chromium.org> Date: Thu Nov 16 01:59:32 2017 [desktopCapture] set expected results when a dialog is cancelled. The desktop capture has the shema[1] that has two parameters, a streamId and an option object which is added laterly. But when we cancle the picker dialog, the option object isn't added into the result in Cancel()[2]. So schemaUtils.validate() will throw an error due to the violation. This CL addes an option object into the result list to meet the requirement. [1]: https://cs.chromium.org/chromium/src/chrome/common/extensions/api/desktop_capture.json?l=38 [2]: https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc?l=73 Bug: 784508 Change-Id: Ie17743dacff5f0171af52445df673b70e89a62cd Reviewed-on: https://chromium-review.googlesource.com/773115 Reviewed-by: Zijie He <zijiehe@chromium.org> Commit-Queue: Weiyong Yao <braveyao@chromium.org> Cr-Commit-Position: refs/heads/master@{#516952} [modify] https://crrev.com/e334368d858f03a8fd653f72eebf37675c16d585/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
,
Nov 16 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by braveyao@chromium.org
, Nov 13 2017Owner: braveyao@chromium.org
Status: Assigned (was: Untriaged)