Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1216)

Issue 1276373004: Revert of Switch media stream permissions to use IsOriginSecure() instead of SchemeIsSecure(). (Closed)

Created:
5 years, 4 months ago by magjed_chromium
Modified:
5 years, 4 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org, felt, mcasas+watch_chromium.org, palmer, posciak+watch_chromium.org, raymes, wjia+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Switch media stream permissions to use IsOriginSecure() instead of SchemeIsSecure(). (patchset #11 id:220001 of https://codereview.chromium.org/1132203002/ ) Reason for revert: This CL broke WebRtcWebcamBrowserTest.TestAcquiringAndReacquiringWebcam on all platforms, failing on the introduced assert 'EXPECT_TRUE(permissionRequestObserver.request_shown());’ at webrtc_browsertest_base.cc:149. Link to bots: https://build.chromium.org/p/chromium.webrtc/builders/Win7%20Tester https://build.chromium.org/p/chromium.webrtc/builders/Linux%20Tester https://build.chromium.org/p/chromium.webrtc/builders/Mac%20Tester Stdio output: [ RUN ] WebRtcWebcamBrowserTests/WebRtcWebcamBrowserTest.TestAcquiringAndReacquiringWebcam/0 [7391:1799:0807/160702:WARNING:vt_video_decode_accelerator.cc(194)] Failed to initialize VideoToolbox framework. Hardware accelerated video decoding will be disabled. [7388:1799:0807/160703:INFO:CONSOLE(71)] "This appears to be Chrome", source: http://127.0.0.1:50232/webrtc/adapter.js (71) [7388:71959:0807/160703:WARNING:embedded_test_server.cc(258)] Request not handled. Returning 404: /favicon.ico [7388:1799:0807/160703:INFO:CONSOLE(13)] "Returning has-video-input-device to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160703:INFO:CONSOLE(13)] "Requesting doGetUserMedia: constraints: {"video":{"mandatory":{"minWidth":640,"maxWidth":640,"minHeight":480,"maxHeight":480}}}", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160703:INFO:CONSOLE(13)] "Returning request-callback-granted to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160703:INFO:CONSOLE(13)] "Returning ok-got-stream to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160703:INFO:CONSOLE(13)] "Returning ok-started to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160703:INFO:CONSOLE(13)] "Returning video-not-playing to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160703:INFO:CONSOLE(13)] "Returning video-playing to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160703:INFO:CONSOLE(13)] "Returning ok-640x480 to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160704:INFO:CONSOLE(13)] "Returning ok-stopped to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160704:INFO:CONSOLE(13)] "Requesting doGetUserMedia: constraints: {"video":{"mandatory":{"minWidth":640,"maxWidth":640,"minHeight":360,"maxHeight":360}}}", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160704:INFO:CONSOLE(13)] "Returning request-callback-granted to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) ../../chrome/browser/media/webrtc_browsertest_base.cc:149: Failure Value of: permissionRequestObserver.request_shown() Actual: false Expected: true [7388:1799:0807/160704:INFO:CONSOLE(13)] "Returning ok-got-stream to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160704:INFO:CONSOLE(13)] "Returning ok-started to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160704:INFO:CONSOLE(13)] "Returning video-not-playing to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160704:INFO:CONSOLE(13)] "Returning video-playing to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160704:INFO:CONSOLE(13)] "Returning ok-640x360 to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [7388:1799:0807/160704:INFO:CONSOLE(13)] "Returning ok-stopped to test.", source: http://127.0.0.1:50232/webrtc/test_functions.js (13) [ FAILED ] WebRtcWebcamBrowserTests/WebRtcWebcamBrowserTest.TestAcquiringAndReacquiringWebcam/0, where GetParam() = "force-qtkit" (2127 ms) Original issue's description: > Switch media stream permissions to use IsOriginSecure() instead of > SchemeIsSecure(). > > In particular, note that this means that media permissions are persisted on http://localhost. > > The change also updates the old media stream permission tests (which > were no longer testing anything meaningful). > > BUG=295723, 362214, 509844 > > Committed: https://crrev.com/2249c80e8c4b50793a8261c4f9eb3f879ddf4aff > Cr-Commit-Position: refs/heads/master@{#342458} TBR=felt@chromium.org,raymes@chromium.org,tommi@chromium.org,lgarron@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=295723, 362214, 509844 Committed: https://crrev.com/05d9f8f40cf146a4f45f469b5e5a43cad7f397ee Cr-Commit-Position: refs/heads/master@{#342562}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -176 lines) Patch
M chrome/browser/media/chrome_media_stream_infobar_browsertest.cc View 7 chunks +25 lines, -66 lines 0 comments Download
M chrome/browser/media/desktop_capture_access_handler.cc View 3 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/media/media_stream_device_permissions.cc View 2 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/media/media_stream_devices_controller.cc View 3 chunks +2 lines, -3 lines 0 comments Download
M chrome/browser/media/media_stream_infobar_delegate.cc View 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/media/webrtc_browsertest_base.h View 2 chunks +0 lines, -7 lines 0 comments Download
M chrome/browser/media/webrtc_browsertest_base.cc View 5 chunks +2 lines, -92 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
magjed_chromium
Created Revert of Switch media stream permissions to use IsOriginSecure() instead of SchemeIsSecure().
5 years, 4 months ago (2015-08-09 15:19:40 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1276373004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1276373004/1
5 years, 4 months ago (2015-08-09 15:19:47 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 4 months ago (2015-08-09 15:20:09 UTC) #3
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/05d9f8f40cf146a4f45f469b5e5a43cad7f397ee Cr-Commit-Position: refs/heads/master@{#342562}
5 years, 4 months ago (2015-08-09 15:20:53 UTC) #4
lgarron
5 years, 4 months ago (2015-08-13 06:01:31 UTC) #5
Message was sent while issue was closed.
On 2015/08/09 at 15:20:53, commit-bot wrote:
> Patchset 1 (id:??) landed as
https://crrev.com/05d9f8f40cf146a4f45f469b5e5a43cad7f397ee
> Cr-Commit-Position: refs/heads/master@{#342562}

It seems this test didn't run on the default trybots. Which one(s) should I run
to sanity check my fix at https://codereview.chromium.org/1286363002 ?

Powered by Google App Engine
This is Rietveld 408576698