[Merge] lp:~abreu-alexandre/webbrowser-app/flexible-lifecycle-click-hook into lp:webbrowser-app

[Alberto Mardegan]

Review: Needs Fixing

Please fix the style of the continuation lines to make it consistent (i.e., don't start a line with a comma).

There's an inline comment in executeHookDirectives() where I suspect you forgot a check.

Also, it's not clear why you changed the test; if you wanted to test one more combination, maybe you should rewrite the test to be data-driven (using QFETCH and friends)?

Diff comments:

> === modified file 'click-hooks/hook-utils.cpp'
> --- click-hooks/hook-utils.cpp	2014-11-19 14:25:36 +0000
> +++ click-hooks/hook-utils.cpp	2015-12-04 16:26:26 +0000
> @@ -36,6 +36,61 @@
>      return components.join('_');
>  }
>  
> +QString stringFromClickLifeCyclePhase(
> +        HookUtils::WebappHookParser::ClickLifeCyclePhase phase)
> +{
> +    using namespace HookUtils;
> +    switch(phase) {
> +    case WebappHookParser::CLICK_LIFECYCLE_PHASE_INSTALL:
> +        return "install";
> +        break;
> +    case WebappHookParser::CLICK_LIFECYCLE_PHASE_UNINSTALL:
> +        return "uninstall";
> +        break;
> +    case WebappHookParser::CLICK_LIFECYCLE_PHASE_UPDATE:
> +        return "update";
> +        break;
> +    }
> +    return QString();
> +}
> +
> +void executeHookDirectives(const QString& hookFilename
> +                           , HookUtils::WebappHookParser::ClickLifeCyclePhase phase) {
> +    QFileInfo fileInfo(hookFilename);
> +    if (!fileInfo.exists() || !fileInfo.isFile())
> +    {
> +        qDebug() << "Cannot execute directives for" << hookFilename;
> +        return;
> +    }
> +
> +    qDebug() << "Handling" << hookFilename;
> +
> +    HookUtils::WebappHookParser webappHookParser;
> +    HookUtils::WebappHookParser::Data data =
> +            webappHookParser.parseContent(
> +                hookFilename,
> +                phase);
> +
> +    QString appIdNoVersion = fileInfo.fileName();
> +
> +    if (data.shouldDeleteCacheOnUninstall)

Shouldn't you do this only if the phase is CLICK_LIFECYCLE_PHASE_UNINSTALL?

> +    {
> +        QDir dir(QStandardPaths::writableLocation(QStandardPaths::GenericCacheLocation)
> +                 + "/" + shortAppIdFromUnversionedAppId(appIdNoVersion));
> +        dir.removeRecursively();
> +
> +        qDebug() << "Removing cache from" << dir.absolutePath();
> +    }
> +    if (data.shouldDeleteCookiesOnUninstall)
> +    {
> +        QDir dir(QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation)
> +                 + "/" + shortAppIdFromUnversionedAppId(appIdNoVersion));
> +        dir.removeRecursively();
> +
> +        qDebug() << "Removing cookies from" << dir.absolutePath();
> +    }
> +}
> +
>  }
>  
>  


-- 
https://code.launchpad.net/~abreu-alexandre/webbrowser-app/flexible-lifecycle-click-hook/+merge/279508
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.