Skip to content

User/aeloros/crashandforeground#1416

Merged
aeloros merged 4 commits intomainfrom
user/aeloros/crashandforeground
Sep 15, 2021
Merged

User/aeloros/crashandforeground#1416
aeloros merged 4 commits intomainfrom
user/aeloros/crashandforeground

Conversation

@aeloros
Copy link
Copy Markdown
Contributor

@aeloros aeloros commented Sep 14, 2021

Ensure all static calls initialize the singleton for the current instance. Fixes #1200

Attempt to transfer foreground rights when redirecting an activation to another instance. Fixes #1439

Updated a useful manual test app to have UI.

@aeloros aeloros self-assigned this Sep 14, 2021
@ghost ghost added the needs-triage label Sep 14, 2021
@aeloros aeloros requested a review from BenJKuhn September 14, 2021 23:23
// Major.Minor version, MinVersion=0 to find any framework package for this major.minor version
const UINT32 c_Version_MajorMinor{ 0x00040001 };
const PACKAGE_VERSION minVersion{};
RETURN_IF_FAILED(MddBootstrapInitialize(c_Version_MajorMinor, nullptr, minVersion));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DrusTheAxe - when you go add your generated versioning headers, please make a pass and find and fix anybody calling Mdd with manual parameters in samples & tests.

Copy link
Copy Markdown
Member

@DrusTheAxe DrusTheAxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should refactor the identity functions plus some minor comments but otherwise LGTM

using namespace std::chrono;

bool IsPackagedProcess()
HWND g_window = NULL;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: HWND g_window{};

Obligatory favorite link ;-) ES.23: Prefer the {}-initializer syntax


bool IsPackagedProcess()
HWND g_window = NULL;
wchar_t g_windowClass[] = L"TestWndClass"; // the main window class name
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: wchar_t g_windowClass[]{ L"TestWndClass"; };

Or even 'const auto' if you prefer

wchar_t g_windowClass[] = L"TestWndClass"; // the main window class name

ATOM _RegisterClass(HINSTANCE hInstance);
BOOL InitInstance(HINSTANCE, int);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing parameter names

(yes, technically not required, but useful and you have the protocol down below)

BOOL InitInstance(HINSTANCE, int);
LRESULT CALLBACK WndProc(HWND, UINT, WPARAM, LPARAM);

std::wstring GetFullIdentityString()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'GetFullIdentityString' is GetCurrentPackageFullName but return as a std::wstring?

Even better if added to dev\common\AppModel.Identity.h

namespace appModel::Identity
{
inline bool IsPackagedProcess()
{
    ...
}

inline std::wstring GetCurrentPackageFullName()
{
    WCHAR packageFullName[PACKAGE_FULL_NAME_MAX_LENGTH + 1]{};
    UINT32 n{ static_cast<UINT32>(ARRAYSIZE(packageFullName)) };
    const auto rc{ ::GetCurrentPackageFullName(&n, packageFullName) };
    if (rc == APPMODEL_ERROR_NO_PACKAGE)
    {
        return std::wstring();
    }
    THROW_IF_WIN32_ERROR(rc);
    return std::wstring(packageFullName);
}
...

return identityString;
}

bool HasIdentity()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove and changed callers to

#include <appmodel.identity.h>
...
if (AppModel::Identity::IsPackagedProcess())

{ OnActivated(sender, args); }
);

auto hInstance = GetModuleHandle(NULL);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: nullptr

// Enqueue the request and signal the other instance.
// Enqueue the request and transfer foreground rights.
EnqueueRedirectionRequestId(id);
AllowSetForegroundWindow(m_processId);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you don't care if this succeeds or not? (ignoring BOOl return value)

@aeloros
Copy link
Copy Markdown
Contributor Author

aeloros commented Sep 15, 2021

/azp run

@aeloros aeloros enabled auto-merge (squash) September 15, 2021 01:27
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@aeloros
Copy link
Copy Markdown
Contributor Author

aeloros commented Sep 15, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@aeloros aeloros merged commit 84a36a4 into main Sep 15, 2021
@aeloros aeloros deleted the user/aeloros/crashandforeground branch September 15, 2021 19:22
aeloros added a commit that referenced this pull request Oct 18, 2021
* some fixes

* update manual test to handle UI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants