URL Dispatcher

Merge lp://staging/~ted/url-dispatcher/bad-url-prompt into lp://staging/url-dispatcher/16.10

  1. bad-url-prompt
  2. Merge into trunk.16.10
Proposed by Ted Gould
Status: Merged
Approved by: Ted Gould
Approved revision: 113
Merged at revision: 94
Proposed branch: lp://staging/~ted/url-dispatcher/bad-url-prompt
Merge into: lp://staging/url-dispatcher/16.10
Prerequisite: lp://staging/~ted/url-dispatcher/ual-snappy-autopkgtest
Diff against target: 740 lines (+352/-72)
24 files modified
CMakeLists.txt (+1/-0)
data/CMakeLists.txt (+10/-0)
data/bad-url.qml (+42/-0)
debian/apparmor/url-dispatcher-bad-url-helper (+15/-0)
debian/control (+4/-0)
debian/rules (+1/-0)
debian/url-dispatcher.install (+3/-0)
po/CMakeLists.txt (+39/-0)
po/POTFILES.in (+1/-0)
po/genpotfiles.sh (+6/-0)
service/CMakeLists.txt (+5/-12)
service/bad-url/CMakeLists.txt (+14/-0)
service/bad-url/exec-tool.c (+42/-0)
service/dispatcher.c (+4/-0)
service/overlay-tracker-iface.h (+1/-0)
service/overlay-tracker-mir.cpp (+69/-39)
service/overlay-tracker-mir.h (+22/-8)
service/overlay-tracker.cpp (+9/-0)
service/overlay-tracker.h (+1/-0)
service/url-overlay/CMakeLists.txt (+14/-0)
tests/dispatcher-test.cc (+1/-0)
tests/overlay-tracker-mock.h (+6/-0)
tests/overlay-tracker-test.cpp (+41/-12)
tests/test-config.h.in (+1/-1)
To merge this branch: bzr merge lp://staging/~ted/url-dispatcher/bad-url-prompt
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
unity-api-1-bot continuous-integration Approve
Review via email: mp+303290@code.staging.launchpad.net

This proposal supersedes a proposal from 2016-08-18.

Commit message

Show error dialog over apps that use Bad URLs

To post a comment you must log in.
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:113
https://jenkins.canonical.com/unity-api-1/job/lp-url-dispatcher-ci/14/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/411/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/417/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-url-dispatcher-ci/14/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:113
https://jenkins.canonical.com/unity-api-1/job/lp-url-dispatcher-ci/15/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/615/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/621
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/444
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/444/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/444
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/444/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/444
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/444/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/444
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/444/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/444
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/444/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/444
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/444/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/444
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/444/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/444/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/444
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/444/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-url-dispatcher-ci/15/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:113
https://jenkins.canonical.com/unity-api-1/job/lp-url-dispatcher-ci/26/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/828
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/835
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/638
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/638/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/638
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/638/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/638
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/638/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/638
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/638/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/638
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/638/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/638
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/638/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/638
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/638/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/638
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/638/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/638
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/638/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-url-dispatcher-ci/26/rebuild

review: Approve (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

Overall very good, took me awhile to get my head around it.

A few trivial/linty optionals inline.

review: Approve
Revision history for this message
Ted Gould (ted) wrote :

On Wed, 2016-10-26 at 20:22 +0000, Charles Kerr wrote:
> > + * Copyright © 2015 Canonical Ltd.
> Wrong year. BLOCKER!!

You should check bzr blame before saying that, this branch has been
around a while ;-)

> > +# Builds the binary translations catalog for each language
> > +# it finds source translations (*.po) for
> > +foreach(POFILE ${POFILES})
> > +    string(REPLACE ".po" "" LANG ${POFILE})
> > +    list(APPEND PO_FILES "${POFILE}")
> > +    gettext_process_po_files(${LANG} ALL PO_FILES "${POFILE}")
> > +    set(INSTALL_DIR
> > ${CMAKE_INSTALL_LOCALEDIR}/${LANG}/LC_MESSAGES)
> > +    install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${LANG}.gmo
> > +            DESTINATION ${INSTALL_DIR}
> > +            RENAME ${GETTEXT_PACKAGE}.mo
> > +    )
> > +endforeach(POFILE)
> This looks like it should work, so if there are compelling reasons to
> reinvent the wheel here that's fine.
>
> But before we land this, consider reusing existing code that does it,
> eg our cmake Intltool module. See indicator-network/po/CMakeLists.txt
> for an example

I don't remember the issue today, but I couldn't get that to work and
was told it is slightly broken. Happy to change, but want something
that works :-)

> > + g_debug("Setting up over lay for PID %d with
> > '%s'", (int)pid, data.appid.c_str());
> trivial: int(pid), here and below
>

Fixed r114

> > +void
> > +OverlayTrackerMir::removeSession (const std::string &type,
> > MirPromptSession * session)
> > +{
> > + g_debug("Removing session: %p", (void*)session);
> > +
> > + for (auto it = ongoingSessions[type].begin(); it !=
> > ongoingSessions[type].end(); it++) {
> > + if (it->session.get() == session) {
> > + ubuntu_app_launch_stop_multiple_helper(typ
> > e.c_str(), it->appid.c_str(), it->instanceid.c_str());
> > + ongoingSessions[type].erase(it);
> We lookup in the ongoingSessions map three times here, better to do
>
> auto& session = ongoingSessions[type];
> for (auto it = session.begin(); it != session.end(); ++it) {
>     ...
>     session.erase(it);

Good point. Fixed r115.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches