Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Implement Brackets Portable Build on Mac and Windows #333

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Implement Brackets Portable Build on Mac and Windows #333

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
989351a
implement portable Brackets on Windows
bchintx Sep 11, 2013
502e06f
simplify portable install filename
bchintx Sep 13, 2013
8c0779a
implement portable install on Mac
bchintx Sep 17, 2013
b722bd8
re-indenting with spaces
bchintx Sep 18, 2013
ce581d2
re-indenting with spaces
bchintx Sep 18, 2013
a08c88f
Merge remote-tracking branch 'origin/bchin/portable-build' into bchin…
bchintx Sep 18, 2013
4549c80
merging latest master and porting portable changes into darkshell cla…
bchintx Sep 20, 2013
698f6f6
refactoring
bchintx Sep 20, 2013
92c9415
fixing merge from master on Mac
bchintx Sep 20, 2013
97e10b6
Merge remote-tracking branch 'origin/master' into bchin/portable-build
bchintx Sep 20, 2013
9e6c826
merging latest master changes
bchintx Jun 27, 2014
4800e8b
Merge remote-tracking branch 'origin/master' into bchin/portable-build
bchintx Jul 31, 2014
080a162
changes per code review
bchintx Jul 31, 2014
0537c9f
fixing Mac implmentation w/ code review requested changes
bchintx Aug 12, 2014
dcd0165
correcting Windows build error
bchintx Aug 12, 2014
ab57b8a
Merge remote-tracking branch 'origin/master' into bchin/portable-build
bchintx Dec 19, 2014
0bcc6ae
changes per code review
bchintx Dec 19, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 106 additions & 33 deletions appshell/cef_main_window.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "resource.h"
#include "native_menu_model.h"
#include "config.h"
#include <sstream>

// external
extern HINSTANCE hInst;
Expand Down Expand Up @@ -104,21 +105,17 @@ BOOL cef_main_window::Create()
{
RegisterWndClass();

int left = CW_USEDEFAULT;
int top = CW_USEDEFAULT;
int width = CW_USEDEFAULT;
int height = CW_USEDEFAULT;
int showCmd = SW_SHOW;

LoadWindowRestoreRect(left, top, width, height, showCmd);
LoadWindowRestoreRect(showCmd);

DWORD styles = WS_OVERLAPPEDWINDOW | WS_CLIPCHILDREN | WS_EX_COMPOSITED;

if (showCmd == SW_MAXIMIZE)
styles |= WS_MAXIMIZE;

if (!cef_host_window::Create(::kWindowClassname, GetBracketsWindowTitleText(),
styles, left, top, width, height))
if (!cef_host_window::Create(::kWindowClassname, GetBracketsWindowTitleText(), styles,
mWindowRect.left, mWindowRect.top, mWindowRect.right, mWindowRect.bottom))
{
return FALSE;
}
Expand Down Expand Up @@ -296,41 +293,125 @@ void cef_main_window::SaveWindowRestoreRect()
RECT rect;
if (GetWindowRect(&rect))
{
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefLeft, rect.left);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefTop, rect.top);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefWidth, ::RectWidth(rect));
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefHeight, ::RectHeight(rect));
mWindowRect.left = rect.left;
mWindowRect.top = rect.top;
mWindowRect.right = ::RectWidth(rect);
mWindowRect.bottom = ::RectHeight(rect);
}

if (wp.showCmd == SW_MAXIMIZE)
{
// When window is maximized, we also store the "restore" size
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefRestoreLeft, wp.rcNormalPosition.left);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefRestoreTop, wp.rcNormalPosition.top);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefRestoreRight, wp.rcNormalPosition.right);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefRestoreBottom, wp.rcNormalPosition.bottom);
mRestoredRect.left = wp.rcNormalPosition.left;
mRestoredRect.top = wp.rcNormalPosition.top;
mRestoredRect.right = wp.rcNormalPosition.right;
mRestoredRect.bottom = wp.rcNormalPosition.bottom;
}

// Maximize is the only special case we handle
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefShowState, (wp.showCmd == SW_MAXIMIZE) ? SW_MAXIMIZE : SW_SHOW);
if (!ClientApp::IsPortableInstall())
{
// for normal installations, serialize to the Registry
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefLeft, mWindowRect.left);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefTop, mWindowRect.top);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefWidth, mWindowRect.right);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefHeight, mWindowRect.bottom);

::WriteRegistryInt(::kWindowPostionFolder, ::kPrefRestoreLeft, mRestoredRect.left);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefRestoreTop, mRestoredRect.top);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefRestoreRight, mRestoredRect.right);
::WriteRegistryInt(::kWindowPostionFolder, ::kPrefRestoreBottom, mRestoredRect.bottom);

::WriteRegistryInt(::kWindowPostionFolder, ::kPrefShowState, (wp.showCmd == SW_MAXIMIZE) ? SW_MAXIMIZE : SW_SHOW);
}
else
{
// for portable installations, serialize to a file in the app folder
std::wstring filename = ClientApp::AppGetSupportDirectory();
filename += L"\\lastWindowState.dat";
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we could we just store this data in the "makePortable" file...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm... that's actually a really good idea. I can change that.

HANDLE hFile = ::CreateFile(filename.c_str(), GENERIC_WRITE,
FILE_SHARE_READ, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
if (INVALID_HANDLE_VALUE != hFile)
{
DWORD dwBytesWritten;
std::stringstream streambuf;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you just write it as a binary file by calling WriteFile((LPVOID)&mMyData, sizeof(mMyData), &dwBytesWritten, NULL);

myData would just be some struct that is constructed to hold all of the data so that it isn't dependent on the internal Windows data structures (in case they change)

Then you won't need the stream i/o library, it's faster and less error prone. Otherwise, you need to write some error checking during the read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another great idea. I'll make that change.

streambuf << mWindowRect.left << ' ';
streambuf << mWindowRect.top << ' ';
streambuf << mWindowRect.right << ' ';
streambuf << mWindowRect.bottom << ' ';
streambuf << mRestoredRect.left << ' ';
streambuf << mRestoredRect.top << ' ';
streambuf << mRestoredRect.right << ' ';
streambuf << mRestoredRect.bottom << ' ';
streambuf << wp.showCmd;
std::string contents = streambuf.str();
::WriteFile(hFile, contents.c_str(), contents.length(), &dwBytesWritten, NULL);

::CloseHandle(hFile);
}
}
}
}
}

// Initialization helper --
// Loads the restore window placement data from the registry
void cef_main_window::LoadWindowRestoreRect(int& left, int& top, int& width, int& height, int& showCmd)
void cef_main_window::LoadWindowRestoreRect(int& showCmd)
{
::GetRegistryInt(::kWindowPostionFolder, ::kPrefLeft, NULL, left);
::GetRegistryInt(::kWindowPostionFolder, ::kPrefTop, NULL, top);
::GetRegistryInt(::kWindowPostionFolder, ::kPrefWidth, NULL, width);
::GetRegistryInt(::kWindowPostionFolder, ::kPrefHeight, NULL, height);
::GetRegistryInt(::kWindowPostionFolder, ::kPrefShowState, NULL, showCmd);
// set defaults
::SetRect(&mWindowRect, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT);
showCmd = SW_SHOW;
::SetRect(&mRestoredRect, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT);

if (!ClientApp::IsPortableInstall())
{
// for normal installations, serialize to the Registry
::GetRegistryInt(::kWindowPostionFolder, ::kPrefLeft, NULL, (int&)mWindowRect.left);
::GetRegistryInt(::kWindowPostionFolder, ::kPrefTop, NULL, (int&)mWindowRect.top);
// stuff the width and height into the right and bottom fields
::GetRegistryInt(::kWindowPostionFolder, ::kPrefWidth, NULL, (int&)mWindowRect.right);
::GetRegistryInt(::kWindowPostionFolder, ::kPrefHeight, NULL, (int&)mWindowRect.bottom);

::GetRegistryInt(::kWindowPostionFolder, ::kPrefShowState, NULL, showCmd);

::GetRegistryInt(::kWindowPostionFolder, ::kPrefRestoreLeft, NULL, (int&)mRestoredRect.left);
::GetRegistryInt(::kWindowPostionFolder, ::kPrefRestoreTop, NULL, (int&)mRestoredRect.top);
::GetRegistryInt(::kWindowPostionFolder, ::kPrefRestoreRight, NULL, (int&)mRestoredRect.right);
::GetRegistryInt(::kWindowPostionFolder, ::kPrefRestoreBottom, NULL, (int&)mRestoredRect.bottom);
}
else
{
// for portable installations, serialize to a file in the app folder
std::wstring filename = ClientApp::AppGetSupportDirectory();
filename += L"\\lastWindowState.dat";
HANDLE hFile = ::CreateFile(filename.c_str(), GENERIC_READ,
FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
if (INVALID_HANDLE_VALUE != hFile)
{
DWORD dwFileSize = ::GetFileSize(hFile, NULL);
DWORD dwBytesRead;
BYTE* buffer = (BYTE*)malloc(dwFileSize);
if (buffer && ::ReadFile(hFile, buffer, dwFileSize, &dwBytesRead, NULL))
{
std::string contents((char*)buffer, dwBytesRead);
std::stringstream streambuf(contents);
streambuf >> mWindowRect.left;
Copy link
Contributor

Choose a reason for hiding this comment

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

see note above. If the buffer doesn't contain enough data, invalid data, etc... then stream i/o will throw an exception. You can avoid that by reading/writing the binary structure and it isn't user editable in that case. You would need to create a struct to store all of this data (plus a signature and version to handle future changes) but that's not much more work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. good point.

streambuf >> mWindowRect.top;
streambuf >> mWindowRect.right;
streambuf >> mWindowRect.bottom;
streambuf >> mRestoredRect.left;
streambuf >> mRestoredRect.top;
streambuf >> mRestoredRect.right;
streambuf >> mRestoredRect.bottom;
streambuf >> showCmd;
}
::CloseHandle(hFile);
}
}
}

// Initialization helper --
// Loads the Restores data and positions the window in its previously saved state
void cef_main_window::RestoreWindowPlacement(int showCmd)
void cef_main_window::RestoreWindowPlacement(const int showCmd)
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason to make this param const? ints are passed by value so even if the function modified it -- it would have no impact on the rest of the app.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Practically speaking, yeah, it really doesn't make much difference -- mostly just one of those "Poh-TAY-toe", "Poh-TAH-toe" things. From the perspective of an API consumer, the const modifier helps define the contract of how the parm is used. I'll remove it if it helps though -- ie. didn't really need to change it.

{
if (showCmd == SW_MAXIMIZE)
{
Expand All @@ -346,15 +427,7 @@ void cef_main_window::RestoreWindowPlacement(int showCmd)
wp.ptMaxPosition.x = -1;
wp.ptMaxPosition.y = -1;

wp.rcNormalPosition.left = CW_USEDEFAULT;
wp.rcNormalPosition.top = CW_USEDEFAULT;
wp.rcNormalPosition.right = CW_USEDEFAULT;
wp.rcNormalPosition.bottom = CW_USEDEFAULT;

GetRegistryInt(::kWindowPostionFolder, ::kPrefRestoreLeft, NULL, (int&)wp.rcNormalPosition.left);
GetRegistryInt(::kWindowPostionFolder, ::kPrefRestoreTop, NULL, (int&)wp.rcNormalPosition.top);
GetRegistryInt(::kWindowPostionFolder, ::kPrefRestoreRight, NULL, (int&)wp.rcNormalPosition.right);
GetRegistryInt(::kWindowPostionFolder, ::kPrefRestoreBottom, NULL, (int&)wp.rcNormalPosition.bottom);
wp.rcNormalPosition = mRestoredRect;

// This returns FALSE on failure but not sure what we could do in that case
SetWindowPlacement(&wp);
Expand Down
7 changes: 5 additions & 2 deletions appshell/cef_main_window.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class cef_main_window : public cef_host_window
protected:
// Initalization - Protected Members
void SaveWindowRestoreRect();
void LoadWindowRestoreRect(int& left, int& top, int& width, int& height, int& showCmd);
void RestoreWindowPlacement(int showCmd);
void LoadWindowRestoreRect(int& showCmd);
void RestoreWindowPlacement(const int showCmd);

// Message Handlers
BOOL HandleEraseBackground(HDC hdc);
Expand All @@ -73,5 +73,8 @@ class cef_main_window : public cef_host_window
private:
// Initialization - Private Members
static ATOM RegisterWndClass();

RECT mWindowRect; // persisted size of normal window
RECT mRestoredRect; // persisted restored size of maximized window
};

2 changes: 2 additions & 0 deletions appshell/client_app.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class ClientApp : public CefApp,
double GetElapsedMilliseconds();
CefString GetCurrentLanguage();
std::string GetExtensionJSSource();
static bool IsPortableInstall();
static CefString AppGetAppDirectory();
static CefString AppGetSupportDirectory();
static CefString AppGetDocumentsDirectory();

Expand Down
48 changes: 43 additions & 5 deletions appshell/client_app_mac.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,50 @@
return result;
}

// check if this is a portable installation
// which is marked by the existence of a file named "makePortable" in the same folder as .app.
bool ClientApp::IsPortableInstall()
{
std::string filename = ClientApp::AppGetAppDirectory();
filename += "/makePortable";
NSString * nsFilename = [NSString stringWithUTF8String:filename.c_str()];
return [[NSFileManager defaultManager] fileExistsAtPath:nsFilename];
}

CefString ClientApp::AppGetSupportDirectory() {
NSString *libraryDirectory = [NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES) objectAtIndex:0];
NSString *supportDirectory = [NSString stringWithFormat:@"%@/%@%@", libraryDirectory, GROUP_NAME, APP_NAME];

return CefString([supportDirectory UTF8String]);
// returns the directory to which the app has been installed
CefString ClientApp::AppGetAppDirectory()
{
// find the path-only of Brackets.app. Need to iterate in case we're called from the helper.app.
NSString * applicationPath = [[NSBundle mainBundle] bundlePath];
NSString * appFilename = [applicationPath lastPathComponent];
while ([appFilename length] && ![appFilename isEqualToString:APP_NAME @".app"])
{
applicationPath = [applicationPath stringByDeletingLastPathComponent];
appFilename = [applicationPath lastPathComponent];
}
applicationPath = [applicationPath stringByDeletingLastPathComponent];

return CefString([applicationPath UTF8String]);
}

CefString ClientApp::AppGetSupportDirectory()
{
if (!IsPortableInstall())
{
// for normal installations, use the Library folder
NSString *libraryDirectory = [NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES) objectAtIndex:0];
NSString *supportDirectory = [NSString stringWithFormat:@"%@/%@%@", libraryDirectory, GROUP_NAME, APP_NAME];
return CefString([supportDirectory UTF8String]);
}
else
{
// for portable installations, use the app's installed folder
std::string strAppDir = ClientApp::AppGetAppDirectory();
strAppDir += "/";
NSString * appDir = [NSString stringWithUTF8String:strAppDir.c_str()];
appDir = [appDir stringByAppendingString:APP_NAME];
return CefString([appDir UTF8String]);
}
}

CefString ClientApp::AppGetDocumentsDirectory() {
Expand Down
62 changes: 58 additions & 4 deletions appshell/client_app_win.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,46 @@ double ClientApp::GetElapsedMilliseconds()
return (timeGetTime() - g_appStartupTime);
}

// returns the directory to which the app has been installed
CefString ClientApp::AppGetAppDirectory()
{
// find the full pathname of the app .exe
std::wstring appPath;
HMODULE hModule = ::GetModuleHandle(NULL);
if (hModule)
{
WCHAR filename[MAX_PATH+1] = {0};
::GetModuleFileName(hModule, filename, MAX_PATH);
appPath = filename;

// strip off the filename and extension
int idx = appPath.rfind('\\');
if (idx >= 0)
appPath = appPath.substr(0, idx);
}

// Convert '\\' to '/'
replace(appPath.begin(), appPath.end(), '\\', '/');

return CefString(appPath);
}

CefString ClientApp::AppGetSupportDirectory()
{
wchar_t dataPath[MAX_UNC_PATH];
SHGetFolderPath(NULL, CSIDL_APPDATA, NULL, SHGFP_TYPE_CURRENT, dataPath);

std::wstring appSupportPath = dataPath;
std::wstring appSupportPath;
if (!IsPortableInstall())
{
// for normal installations, use the user's APPDATA folder
wchar_t dataPath[MAX_UNC_PATH];
SHGetFolderPath(NULL, CSIDL_APPDATA, NULL, SHGFP_TYPE_CURRENT, dataPath);
appSupportPath = dataPath;
}
else
{
// for portable installations, use the app's installed folder
appSupportPath = ClientApp::AppGetAppDirectory();
}

appSupportPath += L"\\" GROUP_NAME APP_NAME;

// Convert '\\' to '/'
Expand All @@ -116,3 +150,23 @@ CefString ClientApp::AppGetDocumentsDirectory()

return CefString(appUserDocuments);
}


// check if this is a portable installation
// to be a portable installation, the installer should write the empty file to the app folder
bool ClientApp::IsPortableInstall()
{
typedef enum { UNINITIALIZED = -1, ISNOTPORTABLE, ISPORTABLE } ePortableFlag;
static ePortableFlag isPortableInstall = UNINITIALIZED;

if (isPortableInstall == UNINITIALIZED)
{
std::wstring filename = ClientApp::AppGetAppDirectory();
filename += L"/makePortable";
Copy link
Contributor

Choose a reason for hiding this comment

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

is "makePortable" the name we want to use? Seems like it should be something like ".portable" Also, this should be shared by both mac and Windows through "config.h" or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just as a quick comment on this point, I've seen programs whose "portable switch" so to speak was a file simply called "portable.dat". I am not sure how this PR works, but if a portable build can be created from a "normal" installation (it does not require a special build/separate download), I would think the file should be named along the lines of just "portable". This way, user's don't have to think "OK, what was that special file name for a portable version again?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when I originally prototyped this implementation, I was just trying to think of something logical sounding -- "makePortable" seemed to make sense. However, I'm open to changing this to either @JeffryBooher's ".portable" or @le717's "portable" suggestion.

HANDLE hFile = ::CreateFile(filename.c_str(), GENERIC_READ,
FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
isPortableInstall = (INVALID_HANDLE_VALUE != hFile) ? ISPORTABLE : ISNOTPORTABLE;
::CloseHandle(hFile);
}
return (isPortableInstall == ISPORTABLE) ? true : false;
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not know C++ so I could be completely wrong here, but wouldn't simply return (isPortableInstall == ISPORTABLE); work here? If the two values equal the result should be true (and likewise false), the same thing you are doing with this ternary operator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove ternary operator.

}