[Workspaces] Implement PWA recognition, launch. #35913

donlaci · 2024-11-13T15:56:16Z

Summary of the Pull Request

With this code addition the Workspaces will be able to handle PWA applications installed in Edge or Chrome.
The snapshot tool will detect the running PWA apps, get their PwaAppId, which will be stored in the workspaces.json file.
The Editor (and later the Launcher UI) will show the corresponding app name and the app icon.
The launcher will launch the Pwa apps.

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Tested locally on a scenarios where there were 3 different edge-Pwa-apps, 3 different chrome-Pwa-apps, edge and chrome also and other apss too.

…controls to the editor

…ocesses. Update LauncherUI, Launcher

…search

… missing

src/modules/Workspaces/WorkspacesEditor/Data/ProjectData.cs

src/modules/Workspaces/WorkspacesEditor/Utils/PwaApp.cs

…y (code cleaning) and updating json parser in Launcher

SeraphimaZykova

Great work on identifying and launching PWA apps!
There are a few comments about the code; after addressing these, it will be good to go.

src/modules/Workspaces/WorkspacesLauncherUI/Utils/PwaApp.cs

src/modules/Workspaces/WorkspacesLauncherUI/Utils/PwaHelper.cs

src/modules/Workspaces/WorkspacesSnapshotTool/SnapshotUtils.cpp

src/modules/Workspaces/WorkspacesCsharpLibrary/WorkspacesCsharpLibrary.csproj

src/modules/Workspaces/WorkspacesEditor/ViewModels/MainViewModel.cs

src/modules/Workspaces/WorkspacesSnapshotTool/SnapshotUtils.cpp

SeraphimaZykova · 2024-11-18T20:00:25Z

src/modules/Workspaces/WorkspacesSnapshotTool/PwaHelper.cpp

+#include <comdef.h>
+#include <Wbemidl.h>
+
+#include <common/utils/elevation.h>
+#include <common/utils/process_path.h>
+#include <common/notifications/NotificationUtil.h>
+
+#include <workspaces-common/WindowEnumerator.h>
+#include <workspaces-common/WindowFilter.h>
+
+#include <WorkspacesLib/AppUtils.h>
+#include <tlhelp32.h>
+#include <winternl.h>
+#include <initguid.h>


Please leave only what's used here.

Suggested change

#include <comdef.h>

#include <Wbemidl.h>

#include <common/utils/elevation.h>

#include <common/utils/process_path.h>

#include <common/notifications/NotificationUtil.h>

#include <workspaces-common/WindowEnumerator.h>

#include <workspaces-common/WindowFilter.h>

#include <WorkspacesLib/AppUtils.h>

#include <tlhelp32.h>

#include <winternl.h>

#include <initguid.h>

#include <ShlObj.h>

#include <tlhelp32.h>

#include <winternl.h>

#include <initguid.h>

#include <filesystem>

#include <wil/result_macros.h>

#include <common/logger/logger.h>

#include <common/utils/winapi_error.h>

#pragma comment(lib, "ntdll.lib")

SeraphimaZykova · 2024-11-18T20:02:20Z

src/modules/Workspaces/WorkspacesSnapshotTool/SnapshotUtils.cpp

+#include <tlhelp32.h>
+#include <winternl.h>
+#include <initguid.h>


Unnecessary includes after moving the code to PwaHelper, please remove them.

SeraphimaZykova · 2024-11-18T20:04:57Z

src/modules/Workspaces/WorkspacesSnapshotTool/PwaHelper.cpp

+            DWORD err = GetLastError();
+            Logger::error(L"Failed to open the process, error: {}", err);


This helps to get the error message.

Suggested change

DWORD err = GetLastError();

Logger::error(L"Failed to open the process, error: {}", err);

Logger::error(L"Failed to open the process, error: {}", get_last_error_or_default(GetLastError()));

SeraphimaZykova · 2024-11-19T11:55:49Z

src/modules/Workspaces/WorkspacesSnapshotTool/SnapshotUtils.cpp

+            std::wstring pwaName = L"";
+            if (IsEdge(data.value()))
+            {
+                CoInitialize(NULL);


Calling CoInitialize() and CoUninitialize() repeatedly can introduce significant overhead. These functions are intended for setting up and tearing down the COM library, and repeated calls can be costly in terms of performance. It's enough that we call them once in main.cpp.

SeraphimaZykova · 2024-11-19T12:38:16Z

src/modules/Workspaces/WorkspacesSnapshotTool/PwaHelper.cpp

+    std::map<std::wstring, std::wstring> pwaAumidToAppId;
+    std::vector<std::wstring> chromeAppIds;
+    std::map<std::wstring, std::wstring> pwaAppIdsToAppNames;


Please convert the global variables into class members within the PwaHelper class.

SeraphimaZykova · 2024-11-19T12:41:28Z

src/modules/Workspaces/WorkspacesSnapshotTool/PwaHelper.cpp

+        return pwaHelperProcessIds;
+    }
+
+    void PwaHelper::InitAumidToAppId(DWORD pid)


pid is unused, please remove it.

SeraphimaZykova · 2024-11-19T13:18:23Z

src/modules/Workspaces/WorkspacesSnapshotTool/PwaHelper.cpp

+        IAppResolver_7* AppResolver;
+        hr = CoCreateInstance(CLSID_StartMenuCacheAndAppResolver, NULL, CLSCTX_INPROC_SERVER | CLSCTX_INPROC_HANDLER, IID_IAppResolver_7, reinterpret_cast<void**>(&AppResolver));


Please consider using smart pointers instead.

Suggested change

IAppResolver_7* AppResolver;

hr = CoCreateInstance(CLSID_StartMenuCacheAndAppResolver, NULL, CLSCTX_INPROC_SERVER | CLSCTX_INPROC_HANDLER, IID_IAppResolver_7, reinterpret_cast<void**>(&AppResolver));

wil::com_ptr<IAppResolver_7> appResolver;

hr = CoCreateInstance(CLSID_StartMenuCacheAndAppResolver, NULL, CLSCTX_INPROC_SERVER | CLSCTX_INPROC_HANDLER, IID_IAppResolver_7, reinterpret_cast<void**>(appResolver.put()));

SeraphimaZykova · 2024-11-19T13:22:28Z

src/modules/Workspaces/WorkspacesSnapshotTool/PwaHelper.cpp

+            WCHAR* pszAppId;
+            hr = AppResolver->GetAppIDForProcess(dwProcessId, &pszAppId, NULL, NULL, NULL);
+            if (SUCCEEDED(hr))
+            {
+                *result = std::wstring(pszAppId);
+                CoTaskMemFree(pszAppId);
+            }


Please consider using smart pointers for strings, too.

Suggested change

WCHAR* pszAppId;

hr = AppResolver->GetAppIDForProcess(dwProcessId, &pszAppId, NULL, NULL, NULL);

if (SUCCEEDED(hr))

{

*result = std::wstring(pszAppId);

CoTaskMemFree(pszAppId);

}

wil::unique_cotaskmem_string pszAppId;

hr = appResolver->GetAppIDForWindow(hWnd, &pszAppId, NULL, NULL, NULL);

if (SUCCEEDED(hr))

{

*result = std::wstring(pszAppId.get());

}

SeraphimaZykova · 2024-11-19T13:31:44Z

src/modules/Workspaces/WorkspacesSnapshotTool/PwaHelper.cpp

+            {
+                std::filesystem::path fsPath(path);
+                fsPath /= NonLocalizable::EdgeBase;
+                for (auto& directory : std::filesystem::directory_iterator(fsPath))


Use const auto& whenever you read a value without modifying it.

donlaci and others added 9 commits October 15, 2024 11:42

[Workspaces] PWA: first steps: implement PWA app searcher, add basic …

4ccce8f

…controls to the editor

spell checker

3168e8b

Snapshot tool: adding command line args for edge

15fdbf0

PWA: add icon handling, add launch of PWA

4ca467b

Impllement Aumid getters and comparison to connect PWA windows and pr…

3892ba3

…ocesses. Update LauncherUI, Launcher

Minor fixes, simplifications

5941a5b

Spell checker

1635a1d

Removing manual PWA selection, spell checker

461186a

Merge remote-tracking branch 'origin/main' into Workspaces_PWA

2b9e985

donlaci requested a review from SeraphimaZykova November 13, 2024 15:56

This comment has been minimized.

Sign in to view

Fix merge conflict

2686c57

This comment has been minimized.

Sign in to view

donlaci added 2 commits November 13, 2024 17:29

Trying to convince spell checker, that "PEB" is a correct word.

eeb6b51

XAML format fix

a6d3e4e

crutkas added the Needs-Review This Pull Request awaits the review of a maintainer. label Nov 13, 2024

Extending snapshot tool by logs for better testablility

0752331

This comment has been minimized.

Sign in to view

donlaci added 5 commits November 14, 2024 15:24

spell checker fix

20c0f8c

extending logs

d758fa7

extending logs

1e0872d

Removing some logs, modifying search criteria for pwa helper process …

7d0b589

…search

extending PWA detection for the case the directory with the app-id is…

af2ae83

… missing

SeraphimaZykova reviewed Nov 18, 2024

View reviewed changes

src/modules/Workspaces/WorkspacesEditor/Data/ProjectData.cs Show resolved Hide resolved

Fix issue when pwaAppId is null

dfea224

SeraphimaZykova reviewed Nov 18, 2024

View reviewed changes

src/modules/Workspaces/WorkspacesEditor/Utils/PwaApp.cs Outdated Show resolved Hide resolved

fix missing pwa-app-id handling in the editor. Removed unused propert…

2278956

…y (code cleaning) and updating json parser in Launcher

SeraphimaZykova requested changes Nov 18, 2024

View reviewed changes

src/modules/Workspaces/WorkspacesLauncherUI/Utils/PwaApp.cs Outdated Show resolved Hide resolved

src/modules/Workspaces/WorkspacesLauncherUI/Utils/PwaHelper.cs Outdated Show resolved Hide resolved

src/modules/Workspaces/WorkspacesSnapshotTool/SnapshotUtils.cpp Show resolved Hide resolved

donlaci added 2 commits November 18, 2024 16:26

Code cleaning: Moving duplicate code to a common project

4cbf7a4

Fix issue: adding new Guid as app id if it is empty

383846d

Code cleanup: moving Pwa related code from snapshotUtils to PwaHelper

8ad3ca6

SeraphimaZykova reviewed Nov 18, 2024

View reviewed changes

src/modules/Workspaces/WorkspacesCsharpLibrary/WorkspacesCsharpLibrary.csproj Outdated Show resolved Hide resolved

SeraphimaZykova reviewed Nov 18, 2024

View reviewed changes

src/modules/Workspaces/WorkspacesEditor/ViewModels/MainViewModel.cs Outdated Show resolved Hide resolved

SeraphimaZykova reviewed Nov 18, 2024

View reviewed changes

src/modules/Workspaces/WorkspacesEditor/ViewModels/MainViewModel.cs Outdated Show resolved Hide resolved

SeraphimaZykova reviewed Nov 18, 2024

View reviewed changes

src/modules/Workspaces/WorkspacesSnapshotTool/SnapshotUtils.cpp Show resolved Hide resolved

donlaci added 2 commits November 19, 2024 10:40

Code cleaning

e2230c8

Code cleanup: Move common Application model to Csharp Library

09f9661

SeraphimaZykova requested changes Nov 19, 2024

View reviewed changes

donlaci added 2 commits November 19, 2024 17:02

code cleanup

0359d95

modifying package name

9494e76

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Workspaces] Implement PWA recognition, launch. #35913

[Workspaces] Implement PWA recognition, launch. #35913

donlaci commented Nov 13, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

SeraphimaZykova left a comment

SeraphimaZykova Nov 18, 2024

SeraphimaZykova Nov 18, 2024

SeraphimaZykova Nov 18, 2024

SeraphimaZykova Nov 19, 2024

SeraphimaZykova Nov 19, 2024

SeraphimaZykova Nov 19, 2024

SeraphimaZykova Nov 19, 2024

SeraphimaZykova Nov 19, 2024

SeraphimaZykova Nov 19, 2024

		DWORD err = GetLastError();
		Logger::error(L"Failed to open the process, error: {}", err);

		IAppResolver_7* AppResolver;
		hr = CoCreateInstance(CLSID_StartMenuCacheAndAppResolver, NULL, CLSCTX_INPROC_SERVER \| CLSCTX_INPROC_HANDLER, IID_IAppResolver_7, reinterpret_cast<void**>(&AppResolver));

[Workspaces] Implement PWA recognition, launch. #35913

Are you sure you want to change the base?

[Workspaces] Implement PWA recognition, launch. #35913

Conversation

donlaci commented Nov 13, 2024 • edited Loading

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

SeraphimaZykova left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donlaci commented Nov 13, 2024 •

edited

Loading