webbrowser-app

Merge lp:~uriboni/webbrowser-app/media-access into lp:webbrowser-app

media-access
Merge into trunk

Proposed by Ugo Riboni on 2015-09-30

Status:

Merged

Approved by:

Olivier Tilloy on 2015-10-19

Approved revision:

1222

Merged at revision:

1246

Proposed branch:

lp:~uriboni/webbrowser-app/media-access

Merge into:

lp:webbrowser-app

Diff against target:

709 lines (+508/-13)

9 files modified

debian/webbrowser-app-apparmor.manifest (+2/-0)
src/app/webbrowser/Browser.qml (+30/-0)
src/app/webbrowser/MediaAccessDialog.qml (+69/-0)
src/app/webbrowser/SettingsDeviceSelector.qml (+71/-0)
src/app/webbrowser/SettingsPage.qml (+97/-13)
tests/autopilot/webbrowser_app/emulators/browser.py (+18/-0)
tests/autopilot/webbrowser_app/tests/http_server.py (+10/-0)
tests/autopilot/webbrowser_app/tests/test_media_access_permission.py (+98/-0)
tests/unittests/qml/tst_SettingsPage.qml (+113/-0)

To merge this branch:

bzr merge lp:~uriboni/webbrowser-app/media-access

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Approve on 2015-10-20
Olivier Tilloy		2015-09-30	Approve on 2015-10-19
Review via email: mp+272919@code.launchpad.net

Commit message

Implement support for allowing or denying access to media input devices and for setting default media input devices.

Description of the change

Implement media access permissions support, prompting the user when the page requests access to the microphone or camera, and allowing the user to change their choices via new settings pages

Additionally allow the user to select the default audio and video input devices in the settings.

To test this on desktop if you don't have multiple webcams:

- sudo apt-get install v4l2loopback-dkms v4l2loopback-source
- sudo modprobe v4l2loopback
- gst-launch-0.10 gst-launch-0.10 videotestsrc ! v4l2sink device=/dev/video1

Replace video1 with the highest numbered video device in /dev/video*

Then in the settings you can switch between the sources. However to make the change effective you will need to cause the webpage to call again navigator.getUserMedia to pickup the new default stream(s).

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-09-30:

Thanks for your work on this. I haven’t had a chance to test/review yet, but here is a preliminary comment: if/when https://code.launchpad.net/~osomon/webbrowser-app/apparmor-profile/+merge/272850 lands, we will need to add the 'microphone' and 'camera' policy groups to debian/webbrowser-app-apparmor.manifest.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-09-30:

You’re connecting to onMediaAccessPermissionRequested for the current webview. What happens if another tab requests media access? Is the request lost?

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-01:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2304/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Ugo Riboni (uriboni) wrote on 2015-10-01:

> You’re connecting to onMediaAccessPermissionRequested for the current webview.
> What happens if another tab requests media access? Is the request lost?

If there is no handler connected to mediaAccessPermissionRequested then oxide will reject the request, but it will also not remember that choice internally for the session.
So when you go back to the page and cause navigator.getUserMedia to be called again by the page, a new onMediaAccessPermissionRequested call will be made, and webbrowser-app will be able to handle it.

What I can try to do if this is not the desired behavior is to connect mediaAccessPermissionRequested to all open webviews, and if the user has already granted/denied permission then go ahead and respond with that information to the request. Otherwise I can try to "hold" the request pending and display a dialog with the permission request as soon as the user switches back to the page. Though I am not sure yet if this is possible, because as soon as the mediaAccessPermissionRequested handler returns, oxide will temporarily deny the request.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-01:

I don’t think holding up the request until the tab is focused is a good idea. Can you check how other mobile browsers behave in that regard? I would expect they either discard the request right away, or focus the tab that issues the request to make the user aware of the request.

Revision history for this message

Ugo Riboni (uriboni) wrote on 2015-10-01:

> I don’t think holding up the request until the tab is focused is a good idea.
> Can you check how other mobile browsers behave in that regard? I would expect
> they either discard the request right away, or focus the tab that issues the
> request to make the user aware of the request.

Both firefox and chrome do something similar to what I suggested: they do nothing, and when you switch to the tab that has requested permission they pop up an authorization request. So it seems that internally, they are actually "holding" on to the request until the user has a chance to decide. However I don't think we can do that with the current implementation of Oxide.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-05:

> Both firefox and chrome do something similar to what I suggested: they do
> nothing, and when you switch to the tab that has requested permission they pop
> up an authorization request. So it seems that internally, they are actually
> "holding" on to the request until the user has a chance to decide. However I
> don't think we can do that with the current implementation of Oxide.

In that case discarding the request right away (by not connecting to the signal as you’re doing it) is probably the most sensible option. Can you please add a comment though, to make that explicit?

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-05:

Can the "media access" entry in the settings go before the "reset browser settings" one?

"Media Access" should probably be spelled "Media access" to be consistent with other entries in the settings menu.

What about the informative message about needing to restart the browser for a domain permission to take effect, do we still want to display something like that?

Browsing to https://opentokrtc.com/foobarbaz, allowing both camera and microphone access, then I go to settings and disallow microphone access for that domain, restart the browser and I’m prompted for both permissions again. I would expect not to be prompted at all.

Similarly, if I do the above but instead of disallowing microphone access I forget the permission for this domain (by swiping to delete the entry), and after restarting the browser, I expect that the prompt would request microphone permission only (as camera was already granted). Instead, it prompts for both camera and microphone.

If I click anywhere outside the permission prompt, it is dismissed, but I have no idea whether the permission was granted or not. I think that dialog should be modal (or at least tab-modal) so it can’t be dismissed without explicitly choosing to allow or forbid media access.

review: Needs Fixing (functional)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-05:

I haven’t done a complete code review yet, but here’s a first round of comments:

For completeness, can you add to the class documentation of MediaAccessModel that its entries are not sorted in any particular order? (thanks for the detailed documentation btw, this is much appreciated)

In MediaAccessModel::data(), is the explicit cast to bool really needed?

isNullOrUndefined() should probably be enclosed in an anonymous namespace.

In MediaAccessModel::set(), writes to the DB could be done after signals (dataChanged, rowsInserted, rowCountChanged) have been emitted. This would have the potential to make the UI more responsive by not tying it to disk access (and would be consistent with how other SQLite-DB-backed models behave).
The same remark applies to ::unset().

The docstring for MediaAccessModel::unset() starts with "\qmlmethod void set", it should be "\qmlmethod void unset".

In MediaAccessModel::unset(), if an origin has e.g. only audio set, and I request unsetting only video, it looks to me like a dataChanged signal will be emitted (and the database will be written to), while in fact nothing should happen.

Revision history for this message

Chris Coulson (chrisccoulson) wrote on 2015-10-05:

I've had a brief look and left a comment inline.

The application shouldn't really be storing these decisions - it doesn't work properly for media access in some circumstances anyway because Chromium needs access to these decisions for navigator.mediaDevices.enumerateDevices to work properly (the device names are scrubbed unless permission has already been granted to a domain).

(It's even worse for notifications because a site has to explicitly request permission before sending a notification. This is how we ended up with a hack in Oxide to remember these decisions for the rest of a session).

Site-specific settings really belong in Oxide (see https://blueprints.launchpad.net/oxide/+spec/site-settings, and https://docs.google.com/document/d/1EZV4KzrH9eEUMjh3G8Cdykw3JTBtqOlunEEZd1kothA/edit)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-05:

Although this is functionally fine, I’d prefer the databasePath of MediaAccessModel to be set declaratively rather than imperatively in Component.onCompleted. You could do something like that:

    Binding {
        target: MediaAccessModel
        property: "databasePath"
        value: dataLocation + "/mediaAccess.sqlite"
    }

A media permission request’s 'origin' parameter includes the scheme, and I think this is important information that would should not be trimming when remembering permissions for an origin. A user might want to allow use of the camera for a given domain over an encrypted connection (e.g. origin='https://webrtc.foobar.org/baz') but not over an unencrypted connection (e.g. origin='http://webrtc.foobar.org/bleh').

In MediaAccessDialog.qml, there are extraneous whitespaces between the end of a sentence and the question mark that closes them, in user-facing strings.

In SettingsPage.qml, the import of Qt.labs.settings can now be removed.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-06:

> The application shouldn't really be storing these decisions […]

Thanks for the review and pointers Chris.

So until this API is available in oxide, I guess the way to go for the browser is to pop up a permission request for every emission of mediaAccessPermissionRequested, right?
This would be consistent with the way geolocation permission requests currently work in the browser. Slightly annoying for a user, but it will at least allow using camera and microphone, so definitely an improvement.

Ugo, can you rework that branch (or create a new one) to implement that?

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-06:

The browser is now running under apparmor confinement, so we need to add the 'microphone' and 'camera' policy groups to debian/webbrowser-app-apparmor.manifest.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-06:

And one additional point that Chris raised during our oxide weekly call: remembering permissions should only be done when browsing in public mode, incognito mode should not leave a trace of the user’s actions. No big deal now that this logic is moving to oxide, the incognito check will be handled there.

lp:~uriboni/webbrowser-app/media-access updated on 2015-10-12

1195. By Ugo Riboni on 2015-10-01: Add support for selecting the default audio input device for the web context
1196. By Ugo Riboni on 2015-10-05: Move the media access settings within the privacy section and rename settings items according to design spec
1197. By Ugo Riboni on 2015-10-05: Add comment to explain why we are connecting the mediaAccessPermissionRequested only on the current webview
1198. By Ugo Riboni on 2015-10-06: Add a note warning the user that they will need to restart the application for some changes to the media access permissions to be effective.
1199. By Ugo Riboni on 2015-10-06: Make the dialog an actual dialog, and modal
1200. By Ugo Riboni on 2015-10-06: Clarify and fix some MediaAccessModel documentation
1201. By Ugo Riboni on 2015-10-06: Update text of the restart note to match design
1202. By Ugo Riboni on 2015-10-06: Don't store permissions locally. Ask the user every time oxide sends a request
1203. By Ugo Riboni on 2015-10-12: Pop up a modal dialog when the active tab requests media access.
1204. By Ugo Riboni on 2015-10-12: Clarify via comments why we are doing things this way and what is the plan for the future
1205. By Ugo Riboni on 2015-10-12: Remove no longer used properties. Fix english question mark style
1206. By Ugo Riboni on 2015-10-12: Display information about the embedder (if any)
1207. By Ugo Riboni on 2015-10-12: Merge changes from trunk
1208. By Ugo Riboni on 2015-10-12: Fix text and positioning+size of buttons

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-12:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2347/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-12:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2349/rebuild

review: Needs Fixing (continuous-integration)

lp:~uriboni/webbrowser-app/media-access updated on 2015-10-14

1209. By Ugo Riboni on 2015-10-14: Merge changes from trunk
1210. By Ugo Riboni on 2015-10-14: Add microphone and camera apparmor policy groups

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-14:

In InputDevicesModel.qml, to avoid the double "Oxide." prefix, you can simply import com.canonical.Oxide 1.9 as an unnamed import.

What’s the use of InputDevicesModel.qml? Oxide.available{Audio,Video}CaptureDevices is a list of variants, do we really need to convert that to an array of string ids? Can’t we have direct references to Oxide.available{Audio,Video}CaptureDevices in SettingsDeviceSelector.qml ?

When running the autopilot tests, I’m seeing the following warning in the JS console:
[JS] (:0) getUserMedia() is deprecated on insecure origins, and support will be removed in the future. You should consider switching your application to a secure origin, such as HTTPS. See https://goo.gl/rStTGz for more details.
This is out of scope for this MR, but we need to think about supporting https in our autopilot tests.

webbrowser_app.tests.test_media_access_permission.TestMediaAccessPermission.test_allow fails locally with the following traceback:

Traceback (most recent call last):
  File "/home/osomon/dev/phablet/browser/webbrowser-app/tests/autopilot/webbrowser_app/tests/test_media_access_permission.py", line 52, in test_allow
    self.main_window.wait_until_page_loaded(url)
  File "/home/osomon/dev/phablet/browser/webbrowser-app/tests/autopilot/webbrowser_app/emulators/browser.py", line 45, in wait_until_page_loaded
    webview.url.wait_for(url)
  File "/usr/lib/python3/dist-packages/autopilot/introspection/types.py", line 180, in wait_for
    failure_msg))
AssertionError: After 10.0 seconds test on WebViewImpl.url failed: 'http://test/media/v' != dbus.String('http://test/test2', variant_level=1)

Could it be because I don’t have a webcam plugged into my laptop?

review: Needs Fixing

lp:~uriboni/webbrowser-app/media-access updated on 2015-10-14

1211. By Ugo Riboni on 2015-10-14: Cleaner imports
1212. By Ugo Riboni on 2015-10-14: Remove InputDevicesModel.qml as it is not really useful
1213. By Ugo Riboni on 2015-10-14: Alphabetically order apparmor policies
1214. By Ugo Riboni on 2015-10-14: Skip AP test as we can't guarantee a/v devices are present or test for their presence easily
1215. By Ugo Riboni on 2015-10-14: Use decorator to skip tests

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-14:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2352/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-14:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2353/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-15:

There’s now one minor conflict when merging this branch into the latest trunk, can you please resolve it?

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-15:

Also note that after merging the latest trunk QmlTests::tst_SettingsPage will need to be updated, it fails with the following error:

2: QWARN : QmlTests::UnknownTestFunc() file:///home/osomon/dev/phablet/browser/webbrowser-app/tests/unittests/qml/tst_SettingsPage.qml:45:27: HistoryModelMock is not a type
2: historyModel: HistoryModelMock {
2: ^

lp:~uriboni/webbrowser-app/media-access updated on 2015-10-18

1216. By Ugo Riboni on 2015-10-18: Merge changes from trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-18:

PASSED: Continuous integration, rev:1216
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2371/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4695
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1125
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1125
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1125/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1125
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3788
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4692
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4692/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24318

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2371/rebuild

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-19:

For consistency with the rest of the codebase, can you please remove the trailing semi-colons in JS code embedded in QML?

get_deny_button and get_allow_button are used only to have the tests click those buttons, maybe the methods could be transformed into click_*_button (with a corresponding autopilot.logging.log_action decorator, see other similar methods in browser.py).

While testing this branch on desktop, I realized that having a modal dialog displayed doesn’t inhibit keyboard shortcuts. As this isn’t specific to this branch, I filed bug #1507468 to track the issue, and we can address it separately. Just a FYI.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-19:

The rest of the changes look good. I’ve just tested on arale, and I’m not seeing any video devices listed in the settings (even with oxide 1.10.2 installed from the phablet-team PPA). Have you managed to validate that camera devices are listed on another device?

lp:~uriboni/webbrowser-app/media-access updated on 2015-10-19

1217. By Ugo Riboni on 2015-10-19: Remove semicolons in JS
1218. By Ugo Riboni on 2015-10-19: Refactor AP tests to click the button directly from the emulator

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-19:

There’s still one remaining trailing semi-colon in JS code (in Browser.qml).

The import of "../UrlUtils.js" in MediaAccessDialog.qml is unused. So is "id: dialog".

In MediaAccessDialog.qml, the comment for translators could be more specific (e.g. "requesting access *to the microphone/camera*"). Keep in mind that translators are not expected to go read the code to figure out the context of a string.

In SettingsDeviceSelector.qml, shouldn’t updateDefaultDevice() be called when isAudio changes, too?

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-19:

PASSED: Continuous integration, rev:1218
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2373/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4698
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1127
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1127
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1127/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1127
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3791
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4695
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4695/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24329

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2373/rebuild

review: Approve (continuous-integration)

lp:~uriboni/webbrowser-app/media-access updated on 2015-10-19

1219. By Ugo Riboni on 2015-10-19: Kill all the semicolons
1220. By Ugo Riboni on 2015-10-19: Remove unused imports and ids
1221. By Ugo Riboni on 2015-10-19: Clarify explanation for translators
1222. By Ugo Riboni on 2015-10-19: Update the default device in one more where it is necessary

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-10-19:

LGTM.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-19:

FAILED: Continuous integration, rev:1222
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2375/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4703
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1129
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1129
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1129/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1129
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3795
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4700
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4700/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24341

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2375/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-20:

PASSED: Continuous integration, rev:1222
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2376/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4711
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1130
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1130
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1130/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1130
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3804
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4708
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4708/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24364

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2376/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Ubuntu Phablet Team

Ugo Riboni

to status/vote changes:

Rachel Liu

 === modified file 'debian/webbrowser-app-apparmor.manifest'
 --- debian/webbrowser-app-apparmor.manifest	2015-09-29 20:14:26 +0000
 +++ debian/webbrowser-app-apparmor.manifest	2015-10-19 12:13:40 +0000
@@ -9,10 +9,12 @@
        "policy_groups": [
          "accounts",
          "audio",
++        "camera",
          "content_exchange",
          "content_exchange_source",
          "keep-display-on",
          "location",
++        "microphone",
          "networking",
          "push-notification-client",
          "video",
 === modified file 'src/app/webbrowser/Browser.qml'
 --- src/app/webbrowser/Browser.qml	2015-10-13 12:57:33 +0000
 +++ src/app/webbrowser/Browser.qml	2015-10-19 12:13:40 +0000
@@ -75,6 +75,30 @@
+         }
+     }
++    Connections {
++        target: currentWebview
++
++        /* Note that we are connecting the mediaAccessPermissionRequested signal
++           on the current webview only because we want all the tabs that are not
++           visible to automatically deny the request but emit the signal again
++           if the same origin requests permissions (which is the default
++           behavior in oxide if we don't connect a signal handler), so that we
++           can pop-up a dialog asking the user for permission.
++
++           Design is working on a new component that allows per-tab non-modal
++           dialogs that will allow asking permission to the user without blocking
++           interaction with the rest of the page or the window. When ready all
++           tabs will have their mediaAccessPermissionRequested signal handled by
++           creating one of these new dialogs.
++        */
++        onMediaAccessPermissionRequested: PopupUtils.open(mediaAccessDialogComponent, null, { request: request })
++    }
++
++    Component {
++        id: mediaAccessDialogComponent
++        MediaAccessDialog { }
++    }
++
      actions: [
          Actions.GoTo {
              onTriggered: currentWebview.url = value
@@ -118,6 +142,8 @@
          property string allowOpenInBackgroundTab: settingsDefaults.allowOpenInBackgroundTab
          property bool restoreSession: settingsDefaults.restoreSession
          property int newTabDefaultSection: settingsDefaults.newTabDefaultSection
++        property string defaultAudioDevice
++        property string defaultVideoDevice
          function restoreDefaults() {
              homepage  = settingsDefaults.homepage
@@ -125,6 +151,8 @@
              allowOpenInBackgroundTab = settingsDefaults.allowOpenInBackgroundTab
              restoreSession = settingsDefaults.restoreSession
              newTabDefaultSection = settingsDefaults.newTabDefaultSection
++            defaultAudioDevice = settingsDefaults.defaultAudioDevice
++            defaultVideoDevice = settingsDefaults.defaultVideoDevice
+         }
+     }
@@ -136,6 +164,8 @@
          readonly property string allowOpenInBackgroundTab: "default"
          readonly property bool restoreSession: true
          readonly property int newTabDefaultSection: 0
++        readonly property string defaultAudioDevice: ""
++        readonly property string defaultVideoDevice: ""
+     }
      FocusScope {
 === added file 'src/app/webbrowser/MediaAccessDialog.qml'
 --- src/app/webbrowser/MediaAccessDialog.qml	1970-01-01 00:00:00 +0000
 +++ src/app/webbrowser/MediaAccessDialog.qml	2015-10-19 12:13:40 +0000
@@ -0,0 +1,69 @@
++/*
++ * Copyright 2015 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++import QtQuick 2.4
++import Ubuntu.Components 1.3
++import Ubuntu.Components.Popups 1.3
++
++Dialog {
++    property var request
++    modal: true
++
++    title: request.isForAudio && request.isForVideo ?
++           i18n.tr("Allow this domain to access your camera and microphone?") :
++           (request.isForVideo ? i18n.tr("Allow this domain to access your camera?")
++                               : i18n.tr("Allow this domain to access your microphone?"))
++
++    text: request.embedder.toString() !== request.origin.toString() ?
++          internal.textWhenEmbedded : request.origin
++
++    Row {
++        id: internal
++
++        // TRANSLATORS: %1 is the URL of the site requesting access to camera and/or microphone and %2 is the URL of the site that embeds it
++        readonly property string textWhenEmbedded: i18n.tr("%1 (embedded in %2)")
++                                                   .arg(request.origin).arg(request.embedder)
++        height: units.gu(4)
++        spacing: units.gu(2)
++        anchors.horizontalCenter: parent.horizontalCenter
++
++        Button {
++            id: allowButton
++            objectName: "mediaAccessDialog.allowButton"
++            text: i18n.tr("Yes")
++            color: UbuntuColors.green
++            width: units.gu(14)
++            onClicked: {
++                request.allow()
++                hide()
++            }
++        }
++
++        Button {
++            id: denyButton
++            objectName: "mediaAccessDialog.denyButton"
++            text: i18n.tr("No")
++            color: UbuntuColors.red
++            width: units.gu(14)
++            onClicked: {
++                request.deny()
++                hide()
++            }
++        }
++    }
++}
 === added file 'src/app/webbrowser/SettingsDeviceSelector.qml'
 --- src/app/webbrowser/SettingsDeviceSelector.qml	1970-01-01 00:00:00 +0000
 +++ src/app/webbrowser/SettingsDeviceSelector.qml	2015-10-19 12:13:40 +0000
@@ -0,0 +1,71 @@
++/*
++ * Copyright 2015 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++import QtQuick 2.4
++import Ubuntu.Components 1.3
++import com.canonical.Oxide 1.9
++
++Item {
++    property bool isAudio
++    readonly property int devicesCount: internal.devices.length
++    property alias enabled: selector.enabled
++    property string defaultDevice
++    signal deviceSelected(string id)
++
++    implicitHeight: selector.height + units.gu(1)
++
++    OptionSelector {
++        id: selector
++
++        anchors.top: parent.top
++        anchors.left: parent.left
++        anchors.right: parent.right
++        anchors.margins: units.gu(1)
++        containerHeight: itemHeight * model.length
++
++        model: internal.devices
++        delegate: OptionSelectorDelegate { text: modelData.id }
++        onDelegateClicked: deviceSelected(model[index].id)
++    }
++
++    QtObject {
++        id: internal
++
++        property var devices: isAudio ? Oxide.availableAudioCaptureDevices :
++                                        Oxide.availableVideoCaptureDevices
++
++        function updateDefaultDevice() {
++            for (var i = 0; i < devices.length; i++) {
++                if (defaultDevice === devices[i].id) {
++                    selector.selectedIndex = i
++                    return
++                }
++            }
++        }
++    }
++
++    onDefaultDeviceChanged: internal.updateDefaultDevice()
++    Connections {
++        target: Oxide
++        onAvailableAudioCaptureDevicesChanged: if (isAudio) internal.updateDefaultDevice()
++        onAvailableVideoCaptureDevicesChanged: if (!isAudio) internal.updateDefaultDevice()
++    }
++
++    onIsAudioChanged: internal.updateDefaultDevice()
++}
++
 === modified file 'src/app/webbrowser/SettingsPage.qml'
 --- src/app/webbrowser/SettingsPage.qml	2015-09-01 15:35:40 +0000
 +++ src/app/webbrowser/SettingsPage.qml	2015-10-19 12:13:40 +0000
@@ -20,7 +20,7 @@
  import Qt.labs.settings 1.0
  import Ubuntu.Components 1.3
  import Ubuntu.Components.Popups 1.3
--import Ubuntu.Components.ListItems 1.3 as ListItem
++import Ubuntu.Components.ListItems 1.3 as ListItems
  import Ubuntu.Web 0.2
  import webbrowserapp.private 0.1
@@ -29,7 +29,7 @@
  Item {
      id: settingsItem
--    property Settings settingsObject
++    property QtObject settingsObject
      signal done()
@@ -68,7 +68,7 @@
              width: parent.width
--            ListItem.Subtitled {
++            ListItems.Subtitled {
                  objectName: "searchengine"
                  SearchEngine {
@@ -85,7 +85,7 @@
                  onClicked: searchEngineComponent.createObject(subpageContainer)
+             }
--            ListItem.Subtitled {
++            ListItems.Subtitled {
                  objectName: "homepage"
                  text: i18n.tr("Homepage")
@@ -94,7 +94,7 @@
                  onClicked: PopupUtils.open(homepageDialog)
+             }
--            ListItem.Standard {
++            ListItems.Standard {
                  objectName: "restoreSession"
                  text: i18n.tr("Restore previous session at startup")
@@ -112,7 +112,7 @@
+                 }
+             }
--            ListItem.Standard {
++            ListItems.Standard {
                  objectName: "backgroundTabs"
                  text: i18n.tr("Allow opening new tabs in background")
@@ -131,15 +131,15 @@
+                 }
+             }
--            ListItem.Standard {
++            ListItems.Standard {
                  objectName: "privacy"
--                text: i18n.tr("Privacy")
++                text: i18n.tr("Privacy & permissions")
                  onClicked: privacyComponent.createObject(subpageContainer)
+             }
--            ListItem.Standard {
++            ListItems.Standard {
                  objectName: "reset"
                  text: i18n.tr("Reset browser settings")
@@ -186,7 +186,7 @@
                      model: searchEngines.engines
--                    delegate: ListItem.Standard {
++                    delegate: ListItems.Standard {
                          objectName: "searchEngineDelegate_" + index
                          SearchEngine {
                              id: searchEngineDelegate
@@ -224,7 +224,7 @@
                  SettingsPageHeader {
                      id: privacyTitle
                      onBack: privacyItem.destroy()
--                    text: i18n.tr("Privacy")
++                    text: i18n.tr("Privacy & permissions")
+                 }
                  Flickable {
@@ -243,7 +243,13 @@
                          id: privacyCol
                          width: parent.width
--                        ListItem.Standard {
++                        ListItems.Standard {
++                            objectName: "privacy.mediaAccess"
++                            text: i18n.tr("Camera & microphone")
++                            onClicked: mediaAccessComponent.createObject(subpageContainer)
++                        }
++
++                        ListItems.Standard {
                              objectName: "privacy.clearHistory"
                              text: i18n.tr("Clear Browsing History")
                              enabled: HistoryModel.count > 0
@@ -253,7 +259,7 @@
+                             }
+                         }
--                        ListItem.Standard {
++                        ListItems.Standard {
                              objectName: "privacy.clearCache"
                              text: i18n.tr("Clear Cache")
                              onClicked: {
@@ -359,5 +365,83 @@
+             }
+         }
+     }
++
++    Component {
++        id: mediaAccessComponent
++
++        Item {
++            id: mediaAccessItem
++            objectName: "mediaAccessSettings"
++            anchors.fill: parent
++
++            Rectangle {
++                anchors.fill: parent
++                color: "#f6f6f6"
++            }
++
++            SettingsPageHeader {
++                id: mediaAccessTitle
++
++                onBack: mediaAccessItem.destroy()
++                text: i18n.tr("Camera & microphone")
++            }
++
++            Flickable {
++                anchors {
++                    top: mediaAccessTitle.bottom
++                    left: parent.left
++                    right: parent.right
++                    bottom: parent.bottom
++                }
++
++                clip: true
++
++                contentHeight: mediaAccessCol.height
++
++                Column {
++                    id: mediaAccessCol
++                    width: parent.width
++
++                    ListItems.Standard {
++                        text: i18n.tr("Microphone")
++                    }
++
++                    SettingsDeviceSelector {
++                        anchors.left: parent.left
++                        anchors.right: parent.right
++
++                        isAudio: true
++                        visible: devicesCount > 0
++                        enabled: devicesCount > 1
++
++                        defaultDevice: settings.defaultAudioDevice
++                        onDeviceSelected: {
++                            SharedWebContext.sharedContext.defaultAudioCaptureDeviceId = id
++                            settings.defaultAudioDevice = id
++                        }
++                    }
++
++                    ListItems.Standard {
++                        text: i18n.tr("Camera")
++                    }
++
++                    SettingsDeviceSelector {
++                        anchors.left: parent.left
++                        anchors.right: parent.right
++
++                        isAudio: false
++                        visible: devicesCount > 0
++                        enabled: devicesCount > 1
++
++                        defaultDevice: settings.defaultVideoDevice
++                        onDeviceSelected: {
++                          SharedWebContext.sharedContext.defaultVideoCaptureDeviceId = id
++                          settings.defaultVideoDevice = id
++                        }
++                    }
++                }
++            }
++        }
++    }
+ }
 === modified file 'tests/autopilot/webbrowser_app/emulators/browser.py'
 --- tests/autopilot/webbrowser_app/emulators/browser.py	2015-10-08 15:26:18 +0000
 +++ tests/autopilot/webbrowser_app/emulators/browser.py	2015-10-19 12:13:40 +0000
@@ -123,6 +123,9 @@
      def get_http_auth_dialog(self):
          return self.wait_select_single(HttpAuthenticationDialog)
++    def get_media_access_dialog(self):
++        return self.wait_select_single(MediaAccessDialog)
++
      def get_tabs_view(self):
          return self.wait_select_single(TabsList, visible=True)
@@ -402,6 +405,21 @@
          return self.select_single("TextField", objectName="password")
++class MediaAccessDialog(uitk.UbuntuUIToolkitCustomProxyObjectBase):
++
++    @autopilot.logging.log_action(logger.info)
++    def click_deny_button(self):
++        button = self.select_single("Button",
++                                    objectName="mediaAccessDialog.denyButton")
++        self.pointing_device.click_object(button)
++
++    @autopilot.logging.log_action(logger.info)
++    def click_allow_button(self):
++        button = self.select_single("Button",
++                                    objectName="mediaAccessDialog.allowButton")
++        self.pointing_device.click_object(button)
++
++
  class TabPreview(uitk.UbuntuUIToolkitCustomProxyObjectBase):
      @autopilot.logging.log_action(logger.info)
 === modified file 'tests/autopilot/webbrowser_app/tests/http_server.py'
 --- tests/autopilot/webbrowser_app/tests/http_server.py	2015-09-15 15:21:15 +0000
 +++ tests/autopilot/webbrowser_app/tests/http_server.py	2015-10-19 12:13:40 +0000
@@ -193,6 +193,16 @@
                      self.send_auth_request()
              else:
                  self.send_auth_request()
++        elif self.path.startswith("/media/"):
++            self.send_response(200)
++            permissions = self.path[len("/media/"):]
++            self.send_html(
++                "<script>navigator.webkitGetUserMedia("
++                "{video: " + ("true" if "v" in permissions else "false") +
++                ", audio: " + ("true" if "a" in permissions else "false") +
++                "}, function() { location.href = '/test1' } " +
++                ", function() { location.href = '/test2' })</script>"
++            )
          else:
              self.send_error(404)
 === added file 'tests/autopilot/webbrowser_app/tests/test_media_access_permission.py'
 --- tests/autopilot/webbrowser_app/tests/test_media_access_permission.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/webbrowser_app/tests/test_media_access_permission.py	2015-10-19 12:13:40 +0000
@@ -0,0 +1,98 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++#
++# Copyright 2015 Canonical
++#
++# This program is free software: you can redistribute it and/or modify it
++# under the terms of the GNU General Public License version 3, as published
++# by the Free Software Foundation.
++#
++# This program is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU General Public License
++# along with this program. If not, see <http://www.gnu.org/licenses/>.
++
++import testtools
++from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
++
++
++class TestMediaAccessPermission(StartOpenRemotePageTestCaseBase):
++
++    def setUp(self):
++        super(TestMediaAccessPermission, self).setUp()
++        self.url = self.base_url + "/media/"
++        self.allowed_url = self.base_url + "/test1"
++        self.denied_url = self.base_url + "/test2"
++
++    @testtools.skip("We can't guarantee/test that audio/video devices exist")
++    def test_allow(self):
++        # verify that trying to access any media raises an authorization dialog
++        url = self.url + "a"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++        dialog = self.main_window.get_media_access_dialog()
++
++        # note that we have no easy way to verify that the browser actually
++        # grants or denied permission based on our choice, because we can't
++        # easily inspect the contents of the page from AP tests. the simplest
++        # workaround I could find was to redirect the user to two different
++        # pages upon permission granted or denied, and detect that instead
++        dialog.click_allow_button()
++        dialog.wait_until_destroyed()
++        self.main_window.wait_until_page_loaded(self.allowed_url)
++
++        # verify that trying to access the same media for the same origin in
++        # the same session will not ask for permission again...
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(self.allowed_url)
++
++        # ...but it will ask if we try to access other media
++        url = self.url + "v"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++        dialog = self.main_window.get_media_access_dialog()
++        dialog.click_allow_button()
++        dialog.wait_until_destroyed()
++        self.main_window.wait_until_page_loaded(self.allowed_url)
++
++        # now that we granted both permissions, verify that asking for both
++        # together will also not raise the dialog
++        url = self.url + "av"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(self.allowed_url)
++
++    def test_deny(self):
++        # verify that trying to access any media raises an authorization dialog
++        # and we get redirected to the denial page in case we refuse to give
++        # permission
++        url = self.url + "a"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++        dialog = self.main_window.get_media_access_dialog()
++        dialog.click_deny_button()
++        dialog.wait_until_destroyed()
++        self.main_window.wait_until_page_loaded(self.denied_url)
++
++        # verify that trying to access the same media for the same origin in
++        # the same session will not ask for permission again...
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(self.denied_url)
++
++    @testtools.skip("Skipping due to oxide bug, see http://pad.lv/1501017")
++    def test_deny_combined(self):
++        # deny first one input type, then try to ask both and verify that a
++        # request is made for the media that was not asked for the fist time
++        url = self.url + "a"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++        dialog = self.main_window.get_media_access_dialog()
++        dialog.click_deny_button()
++        dialog.wait_until_destroyed()
++        self.main_window.wait_until_page_loaded(self.denied_url)
++
++        url = self.url + "av"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++        dialog = self.main_window.get_media_access_dialog()
 === added file 'tests/unittests/qml/tst_SettingsPage.qml'
 --- tests/unittests/qml/tst_SettingsPage.qml	1970-01-01 00:00:00 +0000
 +++ tests/unittests/qml/tst_SettingsPage.qml	2015-10-19 12:13:40 +0000
@@ -0,0 +1,113 @@
++/*
++ * Copyright 2015 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++import QtQuick 2.4
++import QtTest 1.0
++import Ubuntu.Test 1.0
++import webbrowserapp.private 0.1
++import webbrowsertest.private 0.1
++import "../../../src/app/webbrowser"
++
++Item {
++    width: 400
++    height: 600
++
++    property var settingsPage: settingsPageLoader.item
++
++    Loader {
++        id: settingsPageLoader
++        anchors.fill: parent
++        active: false
++        sourceComponent: SettingsPage {
++            anchors.fill: parent
++
++            // NOTE: the following properties are not necessary for the tests
++            // currently in this file, but if we don't provide them a lot of
++            // warnings will be generated.
++            // Ideally either more tests that use them will be added or the code
++            // in SettingsPage will be refactored to cope with the missing
++            // settings.
++            settingsObject: QtObject {
++                property url homepage
++                property string searchEngine
++                property int newTabDefaultSection: 0
++            }
++        }
++    }
++
++    UbuntuTestCase {
++        name: "TestSettingsPage"
++        when: windowShown
++
++        function clickItem(item) {
++            var center = centerOf(item)
++            mouseClick(item, center.x, center.y)
++        }
++
++        function swipeItemRight(item) {
++            var center = centerOf(item)
++            var dx = item.width * 0.5
++            mousePress(item, center.x, center.y)
++            mouseMoveSlowly(item, center.x, center.y, dx, 0, 10, 0.01)
++            mouseRelease(item, center.x + dx, center.y)
++        }
++
++        function init() {
++            settingsPageLoader.active = true
++            waitForRendering(settingsPageLoader.item)
++        }
++
++        function cleanup() {
++            settingsPageLoader.active = false
++        }
++
++        function getListItems(name, itemName) {
++            var list = findChild(settingsPage, name)
++            var items = []
++            if (list) {
++                // ensure all the delegates are created
++                list.cacheBuffer = list.count * 1000
++
++                // In some cases the ListView might add other children to the
++                // contentItem, so we filter the list of children to include
++                // only actual delegates (names for delegates in this case
++                // follow the pattern "name_index")
++                var children = list.contentItem.children
++                for (var i = 0; i < children.length; i++) {
++                    if (children[i].objectName.indexOf(itemName) == 0) {
++                        items.push(children[i])
++                    }
++                }
++            }
++            return items
++        }
++
++        function activateSettingsItem(itemName, pageName) {
++            var item = findChild(settingsPage, itemName)
++            clickItem(item)
++            var page = findChild(settingsPage, pageName)
++            waitForRendering(page)
++            return page
++        }
++
++        function goToMediaAccessPage() {
++            activateSettingsItem("privacy", "privacySettings")
++            return activateSettingsItem("privacy.mediaAccess", "mediaAccessSettings")
++        }
++    }
++}