Partial fix to 7028 - Pinning in Win7....

Partial fix to 7028 - Pinning in Win7.A complete fix will require Gears to set the application id as a property on the shortcut  As of this cl, web applications hosted by Chrome will appear in their own groups on the task bar.  However, they can not be pinned from the main application window nor can they be pinned from the shortcut.  The former results in Chrome being pinned, and the latter results in a quick start button but does not group web applications under that button.  Instead in the latter case a web appliation will form a new group.

Bug=7028
Test=None.  (When there is a Win7 trybot there may be at test to check if the windows group in the taskbar properly)

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=21595

[brg, 2009-07-24 06:26:37]

Hello,

I saw you were doing something similar with Jumplists so hope you have time to
review.

Thanks!

[Ben Goodger (Google), 2009-07-24 07:20:44]

http://codereview.chromium.org/159336/diff/1/3
File chrome/browser/browser_win.cc (right):

http://codereview.chromium.org/159336/diff/1/3#newcode30
Line 30: if (win_util::GetWinVersion() != win_util::WINVERSION_WIN7) {
nit: no braces.

http://codereview.chromium.org/159336/diff/1/2
File chrome/browser/browser_win.h (right):

http://codereview.chromium.org/159336/diff/1/2#newcode10
Line 10: void SetAppIdForWindow(const std::wstring& app_name, HWND hwnd);
Just put this method in app/win_util.h,cc instead of creating this new file.
Also, add a note explaining its purpose.

[Hironori Bono, 2009-07-24 07:41:27]

brg,

Thank you for your change.
Even though I think it works, I'm a little wondering why this change doesn't
retrieve the address of SetCurrentProcessExplicitAppUserModelID() (*1) and call
it as listed in the following example. (I think it make your change simpler.)

(*1) http://msdn.microsoft.com/en-us/library/dd378422(VS.85).aspx

  typedef HRESULT (__stdcall*
Shell_SetCurrentProcessExplicitAppUserModeID)(const wchar_t*);
  Shell_SetCurrentProcessExplicitAppUserModeID set_user_mode_id =
      reinterpret_cast<Shell_SetCurrentProcessExplicitAppUserModeID>(
      GetProcAddress(module, "SetCurrentProcessExplicitAppUserModelID"));
  if (set_user_mode_id)
    set_user_mode_id(app_name.c_str());

Regards,

Hironori Bono

[brg, 2009-07-24 16:01:51]

Hironori,

RE: SetCurrentProcessExplicitAppUserModelID, although retreival is easier the
behavior it has is not the same as setting the window property.  I believe this
is because the global browser process is always the same, and the renderers have
their own window.  

When I use the process method instead of the window property Chrome.exe without
arguments always stacks with chrome.exe --app=http://myurl.com if called second.

On 2009/07/24 07:41:27, hbono wrote:
> brg,
> 
> Thank you for your change.
> Even though I think it works, I'm a little wondering why this change doesn't
> retrieve the address of SetCurrentProcessExplicitAppUserModelID() (*1) and
call
> it as listed in the following example. (I think it make your change simpler.)
> 
> (*1) http://msdn.microsoft.com/en-us/library/dd378422(VS.85).aspx
> 
>   typedef HRESULT (__stdcall*
> Shell_SetCurrentProcessExplicitAppUserModeID)(const wchar_t*);
>   Shell_SetCurrentProcessExplicitAppUserModeID set_user_mode_id =
>       reinterpret_cast<Shell_SetCurrentProcessExplicitAppUserModeID>(
>       GetProcAddress(module, "SetCurrentProcessExplicitAppUserModelID"));
>   if (set_user_mode_id)
>     set_user_mode_id(app_name.c_str());
> 
> Regards,
> 
> Hironori Bono

[brg, 2009-07-24 16:24:58]

Updated to reflect ben's comments.

[Ben Goodger (Google), 2009-07-24 19:36:16]

LGTM, please land this today if you can before the cutoff, follow up with hbono
afterwards if there are further improvements.

http://codereview.chromium.org/159336/diff/15/1003
File app/win_util.h (right):

http://codereview.chromium.org/159336/diff/15/1003#newcode294
Line 294: void SetAppIdForWindow(const std::wstring& app_name, HWND hwnd);
I would rename the parameter for app_name to app_id or similar and make the
comment not be specific to web applications.. basically different windows in the
same process with unique ids.

[brg, 2009-07-24 19:43:10]

http://codereview.chromium.org/159336/diff/15/1003
File app/win_util.h (right):

http://codereview.chromium.org/159336/diff/15/1003#newcode294
Line 294: void SetAppIdForWindow(const std::wstring& app_name, HWND hwnd);
On 2009/07/24 19:36:16, Ben Goodger wrote:

Done.