Add support for containerURLs through publisher cache folders #2384

aballway · 2017-03-30T21:42:11Z

The reference platform has a concept of an application-group entitlement which allows apps made by the same publisher to share folders and semaphores. The shared folders can be accessed through -[NSFileManager containerURLForSecurityApplicationGroupIdentifier:] which will return an NSURL pointing to the shared folder with the given name if one exists.

These folders are declared in an entitlements file separate from the main .pbxproj and info.plist where other settings are stored, and is pointed at by the value with key CODE_SIGN_ENTITLEMENTS in the PBXNativeTarget's Build Configurations.

Here, the shared folders are implemented using Windows' Publisher Cache Folders, which also allow files to be shared in designated folders between apps of the same publisher, but with the caveat that users can clear the data at any time so it should not be used as the sole storage for key data.

VSImporter has been modified to check if CODE_SIGN_ENTITLEMENTS exists, and if it does, read the file and look for com.apple.security.application-groups, which will contain an array of folder names which are to be shared. The reference platform specifies the format which folders must be named, but Publisher Cache Folders does not have such a restriction for names so any valid folder names will be taken and used.

-[NSFileManager containerURLForSecurityApplicationGroupIdentifier:] is implemented by checking if a Publisher Cache Folder of the given name, and if it does returning an NSURL to it.

Fixes #2295
Fixes #2307

DHowett-MSFT · 2017-03-30T21:57:53Z

Frameworks/Foundation/NSFileManager.mm

+
+#include <COMIncludes.h>
+#import <windows.storage.h>
+#import <wrl\client.h>


standardize on /

DHowett-MSFT · 2017-03-30T21:58:52Z

Frameworks/Foundation/NSFileManager.mm

+    ComPtr<IStorageItem> storageItem;
+    RETURN_NULL_IF_FAILED(folder.As(&storageItem));
+
+    HSTRING folderPath;


You probably want a HStringReference here; i think get_Path will return a +1 HSTRING, and you'll leak it.

Ah, spent too much time with CF, got used to Get = 0

You actually want a Wrappers::HString https://msdn.microsoft.com/en-us/library/hh771141.aspx

HStringReference is for a reference to an existing string resource to avoid copies.

In reply to: 109047974 [](ancestors = 109047974)

bbowman · 2017-03-31T02:14:07Z

tools/vsimporter/include/PBX/PBXNativeTarget.h

    String m_productType;
    StringVec m_buildRuleIds;
    /* End of serialized values */

    PBXFileReference* m_productReferencePtr;
    BuildRuleList m_buildRulePtrs;
+    XCConfigurationList* m_nativeBuildConfigurationListPtr;


XCConfigurationList* [](start = 3, length = 21)

this is fine as a raw ptr? do we not own the config list here?

We do not, it is just a pointer to an object owned by the pbxDoc. Ideally it would be a shared_ptr but that's a lot more fixing than I want to do for this change set.

bbowman · 2017-03-31T02:14:47Z

tools/vsimporter/src/SBProject.cpp

+        extension +=
+            "      </PublisherCacheFolders>\n"
+            "    </Extension>\n";
+        ret.emplace_back(extension);


extension [](start = 25, length = 9)

should you std move extension here?

bbowman · 2017-03-31T02:16:38Z

tools/vsimporter/src/SBProject.cpp

+                pDict = boost::any_cast<Plist::dictionary_type>(&pAny);
+            } catch (const std::exception& e) {
+                SBLog::error() << e.what() << std::endl;
+                pDict = nullptr;


pDict = nullptr; [](start = 12, length = 20)

move this up to the declaration?

bbowman · 2017-03-31T02:20:22Z

rajsesh · 2017-03-31T01:02:09Z

Frameworks/Foundation/NSFileManager.mm


 static const wchar_t* TAG = L"NSFileManager";

+using namespace Microsoft::WRL;
+using namespace ABI::Windows::Storage;
+


we can get rid of the #ifdef linux code here.

rajsesh · 2017-03-31T01:14:50Z

tests/testapps/ApplicationGroupFolderRead/ApplicationGroupFolderRead/ViewController.m

+    // Dispose of any resources that can be recreated.
+}
+
+- (void)readFile {


want to add a negative test here trying to read folder2?

memontic-ms · 2017-03-31T21:38:03Z

tools/vsimporter/src/SBProject.cpp

+            "      </PublisherCacheFolders>\n"
+            "    </Extension>\n";
+        ret.emplace_back(extension);
+    }


Use pugixml for this. See VSTemplateProject.cpp.

memontic-ms · 2017-03-31T21:38:31Z

tools/WinObjC.Tools/vsimporter-templates/WinStore10-App/Package-native.appxmanifest

@@ -43,6 +43,8 @@
    </Application>
  </Applications>

+  $extensions$


$extensions$ [](start = 2, length = 12)

Use in combination with pugixml instead

memontic-ms · 2017-03-31T23:07:52Z

tools/vsimporter/src/PBX/PBXNativeTarget.cpp

+    String ret = "";
+    buildSettings.getValue("CODE_SIGN_ENTITLEMENTS", ret);
+    return ret;
+}


SBResourcesBuildPhase.cpp does this simpler. See SBResourcesBuildPhase::writeVCProjectFiles:
String plistPath = bs.second->getValue("INFOPLIST_FILE");

memontic-ms · 2017-03-31T23:09:06Z

🕐

memontic-ms · 2017-04-03T21:03:35Z

tools/vsimporter/src/vswriter/VSTemplateParameters.cpp

+        extensionsOutput += "  </Extensions>";
+        m_params["$extensions$"] = extensionsOutput;
+    }
+}


You don't seem to be using this function anymore. Revert changes in VSTemplateParameters.cpp/h

memontic-ms · 2017-04-03T21:19:18Z

tools/vsimporter/src/vswriter/VSTemplateProject.cpp

+    Plist::dictionary_type* pDict = nullptr;
+    boost::any pAny;
+    try {
+        Plist::readPlist(entitlementsPath.c_str(), pAny);


Plist::readPlist(entitlementsPath.c_str(), pAny); [](start = 8, length = 49)

Needs to be relative to xcodeproj or something, not relative to the current dir.

msft-Jeyaram · 2017-04-03T21:25:47Z

tools/vsimporter/src/vswriter/VSTemplateProject.cpp

-static void insertUrlSchemes(const String& file, const StringSet& schemes) {
-    //
-    // Inject registered URL schemes into the AppX manifest file, in this format:
+static std::vector<String> getCacheFolders(const String& entitlementsPath) {


getCacheFolders [](start = 27, length = 15)

return this by ref vs making a copy.

The compiler should elide this copy #ByDesign

memontic-ms · 2017-04-04T19:46:44Z

tools/vsimporter/src/vswriter/VSTemplateProject.cpp

+        SBLog::error() << e.what() << std::endl;
+    }
+
+    if (pDict) {


nit: a comment here would be nice

memontic-ms · 2017-04-04T19:47:53Z

tools/vsimporter/src/SBProject.cpp

@@ -16,6 +16,7 @@

 #include <iostream>
 #include <iterator>
+#include <memory>


Remove unused include

memontic-ms · 2017-04-04T19:48:59Z

tools/vsimporter/src/PBX/PBXNativeTarget.cpp

@@ -21,6 +21,8 @@
 #include "PBXBuildRule.h"
 #include "PBXObjectIdConvert.h"
 #include "VariableCollection.h"
+#include "XCBuildConfiguration.h"


Remove unused includes

memontic-ms · 2017-04-04T19:52:15Z

tools/vsimporter/include/SBProject.h

@@ -67,6 +67,7 @@ class SBProject {
    void queryBuildConfigurations();
    void selectBuildConfigurations(const StringSet* configNames);
    void getMatchingFiles(fileMatchFunc matchFunc, ConstFileList& ret) const;
+    std::vector<std::string> SBProject::getPublisherCacheFolderNames() const;


Remove. This is never defined/used.

aballway requested review from DHowett-MSFT, rajsesh, bbowman and memontic-ms March 30, 2017 21:42

msftclas added the cla-not-required label Mar 30, 2017

memontic-ms requested a review from asimonov-msft March 30, 2017 21:50

DHowett-MSFT suggested changes Mar 30, 2017

View reviewed changes

bbowman reviewed Mar 31, 2017

View reviewed changes

rajsesh suggested changes Mar 31, 2017

View reviewed changes

DHowett-MSFT approved these changes Mar 31, 2017

View reviewed changes

rajsesh approved these changes Mar 31, 2017

View reviewed changes

memontic-ms reviewed Mar 31, 2017

View reviewed changes

memontic-ms reviewed Apr 3, 2017

View reviewed changes

msft-Jeyaram reviewed Apr 3, 2017

View reviewed changes

aballway added 2 commits April 3, 2017 15:05

Add support for containerURLs through Publisher Cache Folders

e3b9d03

Use absolute path instead

bd5dd9c

aballway force-pushed the develop branch from 4e67dfc to bd5dd9c Compare April 3, 2017 22:30

memontic-ms reviewed Apr 4, 2017

View reviewed changes

memontic-ms approved these changes Apr 4, 2017

View reviewed changes

Leftover includes; comments

a019baf

aballway merged commit 3438f59 into microsoft:develop Apr 4, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for containerURLs through publisher cache folders #2384

Add support for containerURLs through publisher cache folders #2384

aballway commented Mar 30, 2017

DHowett-MSFT Mar 30, 2017

DHowett-MSFT Mar 30, 2017

aballway Mar 30, 2017

bbowman Mar 31, 2017

bbowman Mar 31, 2017

aballway Mar 31, 2017

bbowman Mar 31, 2017

bbowman Mar 31, 2017

bbowman commented Mar 31, 2017

rajsesh Mar 31, 2017

rajsesh Mar 31, 2017

memontic-ms Mar 31, 2017

memontic-ms Mar 31, 2017

memontic-ms Mar 31, 2017

memontic-ms commented Mar 31, 2017

memontic-ms Apr 3, 2017

memontic-ms Apr 3, 2017

msft-Jeyaram Apr 3, 2017 •

edited

Loading

aballway Apr 3, 2017

memontic-ms Apr 4, 2017

memontic-ms Apr 4, 2017

memontic-ms Apr 4, 2017

memontic-ms Apr 4, 2017

Add support for containerURLs through publisher cache folders #2384

Add support for containerURLs through publisher cache folders #2384

Conversation

aballway commented Mar 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbowman commented Mar 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

memontic-ms commented Mar 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msft-Jeyaram Apr 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msft-Jeyaram Apr 3, 2017 •

edited

Loading