Regression: Unable to share screen through Hangouts video call |
|||||||||||
Issue descriptionVersion: 54.0.2806.0 OS: Ubuntu 14.04 What steps will reproduce the problem? (1) Sign in to Gmail >> Open any chat >> Click on Video call button (2) Now click on 3 dot menu >> Select Screen share button and observe for window to select screen Expected: Window to select screen options should be seen. Actual: Instead no screen share dialog is seen. This is a regression issue broken in M54. Will provide other info soon.
,
Jul 25 2016
Issue is also seen in CrOS with 54.0.2806.0/8631.0.0_Dev falco Good Build: 54.0.2804.0 dev Bad Build: 54.0.2805.0 dev CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/dc5d0632171363ab683aa84754db3b35d6ed39a8..3a5d7f965cc6130fde6dc45dbd9d9504bfa7400a Suspecting https://codereview.chromium.org/2172083002 from changelog. @jbroman: Please confirm the issue and help in re-assigning if it is not related to your change.
,
Jul 25 2016
Seems unlikely to be my CL, but I also don't see anything in the blame range. I can't reproduce it working at any CL in that range locally, though, so I'm not sure if something's wrong with my Chromium builds or not. We're confident the bisect range is correct?
,
Jul 25 2016
Re-running the bisect.
,
Jul 26 2016
For some reason, i am unable to run the successful bisect on Mac OS X. sc00335628@, could you please re-run the bisect and see if that helps? From the actual change log (https://chromium.googlesource.com/chromium/src/+log/54.0.2804.0..54.0.2805.0?pretty=fuller&n=10000): sergeyu@, could you please see if it is related to this change (https://chromium.googlesource.com/chromium/src/+/cc40af91b15bd24efc02f82169ef1b76a5e35dbb) by chance? Thank you!
,
Jul 26 2016
As per Comment#5 re-bisected the issue once again on two different machines and got same bisect info as in comment#2. Did normal bisect and flash bisect too. Chromium invoked all bad builds even after changing the Good and Bad range.
,
Jul 26 2016
OK, I can now reproduce this locally by setting "enable_hangout_services_extension = true" in my gn args. Hangouts uses a hidden component extension ("Hangout Services") to support screen capture.
And I have managed to bisect this to https://chromium.googlesource.com/chromium/src/+/91f655b19888da3f86b57ad8c548da93e7b9aba4, which makes sense since it modifies extension bindings and Hangouts relies on extension bindings for its desktop capture feature.
,
Jul 26 2016
,
Jul 26 2016
I'll have a look. Hopefully there will be some way to fix this in hangouts code since my CL fixes a security vulnerability we really don't want to leave in chrome.
,
Jul 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/748dfbd8dfd66cb299af73f77e0b71cbfdb749af commit 748dfbd8dfd66cb299af73f77e0b71cbfdb749af Author: Antony Sargent <asargent@chromium.org> Date: Tue Jul 26 17:28:16 2016 Revert "Fix extension bindings injection for iframes" This broke hangouts screen sharing ( crbug.com/630928 ) This reverts merge commit d578daf3167747cbfe89cba4c4ca70a88e78651a on the 2785 branch from https://codereview.chromium.org/2183443002/. BUG= 630928 Review URL: https://codereview.chromium.org/2184833002 . Cr-Commit-Position: refs/branch-heads/2785@{#356} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/748dfbd8dfd66cb299af73f77e0b71cbfdb749af/chrome/browser/extensions/extension_bindings_apitest.cc [delete] https://crrev.com/67136956253ac0f224eb33e221815364391a9514/chrome/test/data/extensions/api_test/bindings/external_message_listener/background.js [delete] https://crrev.com/67136956253ac0f224eb33e221815364391a9514/chrome/test/data/extensions/api_test/bindings/external_message_listener/manifest.json [delete] https://crrev.com/67136956253ac0f224eb33e221815364391a9514/chrome/test/data/extensions/api_test/bindings/frames_before_navigation.html [delete] https://crrev.com/67136956253ac0f224eb33e221815364391a9514/chrome/test/data/extensions/api_test/bindings/message_sender/background.js [delete] https://crrev.com/67136956253ac0f224eb33e221815364391a9514/chrome/test/data/extensions/api_test/bindings/message_sender/manifest.json [delete] https://crrev.com/67136956253ac0f224eb33e221815364391a9514/chrome/test/data/extensions/api_test/bindings/message_sender/public.html [modify] https://crrev.com/748dfbd8dfd66cb299af73f77e0b71cbfdb749af/extensions/renderer/script_context.cc [modify] https://crrev.com/748dfbd8dfd66cb299af73f77e0b71cbfdb749af/extensions/renderer/script_context_set.cc
,
Jul 26 2016
Ok, I think I've found the problem. My original CL breaks web page <-> extension messaging in the case where the webpage doing the messaging was opened via "window.open(url, '_blank')". I'm working on a fix now.
,
Aug 1 2016
The original CL (https://codereview.chromium.org/2183443002/) got reverted on trunk due to Dr. Memory failues (https://codereview.chromium.org/2191793002), so this should no longer be an M54 blocker.
,
Aug 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/79b64c3e741cc9c6afbb23885945831a45c6baa5 commit 79b64c3e741cc9c6afbb23885945831a45c6baa5 Author: asargent <asargent@chromium.org> Date: Thu Aug 04 17:17:14 2016 Fix extension bindings injection for iframes (reland) For iframes, we don't want to use the source url for determining the associated extension because it starts out with an about:blank context that is scriptable by its parent. This originally landed in codereview.chromium.org/2151693002/ but was reverted because of bug 630928 as well as the test failing under DrMemory (not with memory errors; just not succeeding which likely indicates some kind of race condition in the test). I've added a fix for bug 630928 but haven't been able to locally reproduce the test failure under DrMemory, so I've added some extra logging to the test to hopefully better understand what might be going wrong. Memory sheriffs: If the FramesExtensionBindingsApiTest.FramesBeforeNavigation test fails again without any actual memory errors, please do not revert the entire CL (since it is an important security fix); instead just disable the test or add it to a suppression file so I can iterate on a fix. BUG= 573131 , 630928 Review-Url: https://codereview.chromium.org/2208483002 Cr-Commit-Position: refs/heads/master@{#409819} [modify] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/chrome/browser/extensions/extension_bindings_apitest.cc [modify] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/chrome/browser/extensions/extension_messages_apitest.cc [add] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/chrome/test/data/extensions/api_test/bindings/external_message_listener/background.js [add] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/chrome/test/data/extensions/api_test/bindings/external_message_listener/manifest.json [add] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/chrome/test/data/extensions/api_test/bindings/frames_before_navigation.html [add] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/chrome/test/data/extensions/api_test/bindings/message_sender/background.js [add] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/chrome/test/data/extensions/api_test/bindings/message_sender/manifest.json [add] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/chrome/test/data/extensions/api_test/bindings/message_sender/public.html [add] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/chrome/test/data/extensions/api_test/messaging/externally_connectable/sites/popup_opener.html [modify] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/extensions/renderer/script_context.cc [modify] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/extensions/renderer/script_context.h [modify] https://crrev.com/79b64c3e741cc9c6afbb23885945831a45c6baa5/extensions/renderer/script_context_set.cc
,
Aug 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c commit 7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c Author: Antony Sargent <asargent@chromium.org> Date: Thu Aug 18 21:51:42 2016 Fix extension bindings injection for iframes (reland) For iframes, we don't want to use the source url for determining the associated extension because it starts out with an about:blank context that is scriptable by its parent. This originally landed in codereview.chromium.org/2151693002/ but was reverted because of bug 630928 as well as the test failing under DrMemory (not with memory errors; just not succeeding which likely indicates some kind of race condition in the test). I've added a fix for bug 630928 but haven't been able to locally reproduce the test failure under DrMemory, so I've added some extra logging to the test to hopefully better understand what might be going wrong. Memory sheriffs: If the FramesExtensionBindingsApiTest.FramesBeforeNavigation test fails again without any actual memory errors, please do not revert the entire CL (since it is an important security fix); instead just disable the test or add it to a suppression file so I can iterate on a fix. BUG= 573131 , 630928 Review-Url: https://codereview.chromium.org/2208483002 Cr-Commit-Position: refs/heads/master@{#409819} (cherry picked from commit 79b64c3e741cc9c6afbb23885945831a45c6baa5) Review URL: https://codereview.chromium.org/2257273002 . Cr-Commit-Position: refs/branch-heads/2785@{#670} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/chrome/browser/extensions/extension_bindings_apitest.cc [modify] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/chrome/browser/extensions/extension_messages_apitest.cc [add] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/chrome/test/data/extensions/api_test/bindings/external_message_listener/background.js [add] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/chrome/test/data/extensions/api_test/bindings/external_message_listener/manifest.json [add] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/chrome/test/data/extensions/api_test/bindings/frames_before_navigation.html [add] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/chrome/test/data/extensions/api_test/bindings/message_sender/background.js [add] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/chrome/test/data/extensions/api_test/bindings/message_sender/manifest.json [add] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/chrome/test/data/extensions/api_test/bindings/message_sender/public.html [add] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/chrome/test/data/extensions/api_test/messaging/externally_connectable/sites/popup_opener.html [modify] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/extensions/renderer/script_context.cc [modify] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/extensions/renderer/script_context.h [modify] https://crrev.com/7ba4c1bf12cc650bacfb38c1acc6e54120e4db4c/extensions/renderer/script_context_set.cc
,
Aug 24 2016
Verified this issue on Windows-10, Ubuntu 14.04 and Mac OS 10.11.6 using chrome latest Beta M53-53.0.2785.80 by following steps mentioned in the original comment. Observed the screen sharing dialog is seen during hangouts call. Hence adding TE-Verified label.
,
Sep 2 2016
,
Oct 13 2016
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by kavvaru@chromium.org
, Jul 25 2016Status: Untriaged (was: Unconfirmed)