Add functionality to the Windows 8 notification display functionality to invoke a caller specified …

Add functionality to the Windows 8 notification display functionality to invoke a caller specified handler function
with a string parameter.

Currently the only consumer of this functionality is the download notification display code which when invoked displays
the downloaded file in the shell.

BUG=151141
R=cpu
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=160988

[robertshield, 2012-10-09 21:17:14]

https://codereview.chromium.org/11096013/diff/9001/chrome/browser/download/do...
File chrome/browser/download/download_status_updater_win.cc (right):

https://codereview.chromium.org/11096013/diff/9001/chrome/browser/download/do...
chrome/browser/download/download_status_updater_win.cc:90: 
nit: blank line

https://codereview.chromium.org/11096013/diff/9001/chrome/browser/download/do...
chrome/browser/download/download_status_updater_win.cc:97: 
nit: blank line

https://codereview.chromium.org/11096013/diff/9001/win8/metro_driver/chrome_a...
File win8/metro_driver/chrome_app_view.cc (right):

https://codereview.chromium.org/11096013/diff/9001/win8/metro_driver/chrome_a...
win8/metro_driver/chrome_app_view.cc:850: #if defined(USE_AURA)
Why is this #if here?

[ananta, 2012-10-09 21:20:53]

https://codereview.chromium.org/11096013/diff/9001/chrome/browser/download/do...
File chrome/browser/download/download_status_updater_win.cc (right):

https://codereview.chromium.org/11096013/diff/9001/chrome/browser/download/do...
chrome/browser/download/download_status_updater_win.cc:90: 
On 2012/10/09 21:17:14, robertshield wrote:
> nit: blank line

Done.

https://codereview.chromium.org/11096013/diff/9001/chrome/browser/download/do...
chrome/browser/download/download_status_updater_win.cc:97: 
On 2012/10/09 21:17:14, robertshield wrote:
> nit: blank line

Done.

https://codereview.chromium.org/11096013/diff/9001/win8/metro_driver/chrome_a...
File win8/metro_driver/chrome_app_view.cc (right):

https://codereview.chromium.org/11096013/diff/9001/win8/metro_driver/chrome_a...
win8/metro_driver/chrome_app_view.cc:850: #if defined(USE_AURA)
On 2012/10/09 21:17:14, robertshield wrote:
> Why is this #if here?

This code is specific to AURA. Will remove it if you feel strongly about it.

[asanka, 2012-10-09 21:34:14]

chrome/browser/download/ LGTM with nit.

https://codereview.chromium.org/11096013/diff/3004/chrome/browser/download/do...
File chrome/browser/download/download_status_updater_win.cc (right):

https://codereview.chromium.org/11096013/diff/3004/chrome/browser/download/do...
chrome/browser/download/download_status_updater_win.cc:133:
download->GetFullPath().value().c_str());
Nit: download->GetTargetFilePath() is preferred.

For a completed download, GetFullPath() and GetTargetFilePath() is the same, but
the canonical method is GetTargetFilePath().

[robertshield, 2012-10-09 21:44:57]

lgtm

https://codereview.chromium.org/11096013/diff/9001/win8/metro_driver/chrome_a...
File win8/metro_driver/chrome_app_view.cc (right):

https://codereview.chromium.org/11096013/diff/9001/win8/metro_driver/chrome_a...
win8/metro_driver/chrome_app_view.cc:850: #if defined(USE_AURA)
On 2012/10/09 21:20:53, ananta wrote:
> On 2012/10/09 21:17:14, robertshield wrote:
> > Why is this #if here?
> 
> This code is specific to AURA. Will remove it if you feel strongly about it.

It's ok to leave it, but please add a brief comment about what it does.

[cpu_(ooo_6.6-7.5), 2012-10-09 21:46:49]

http://codereview.chromium.org/11096013/diff/12003/chrome/browser/download/do...
File chrome/browser/download/download_status_updater_win.cc (right):

http://codereview.chromium.org/11096013/diff/12003/chrome/browser/download/do...
chrome/browser/download/download_status_updater_win.cc:133:
download->GetTargetFilePath().value().c_str());
seems you are passing a pointer to a temporary string on the stack.

[ananta, 2012-10-09 21:58:34]

https://codereview.chromium.org/11096013/diff/9001/win8/metro_driver/chrome_a...
File win8/metro_driver/chrome_app_view.cc (right):

https://codereview.chromium.org/11096013/diff/9001/win8/metro_driver/chrome_a...
win8/metro_driver/chrome_app_view.cc:850: #if defined(USE_AURA)
On 2012/10/09 21:44:57, robertshield wrote:
> On 2012/10/09 21:20:53, ananta wrote:
> > On 2012/10/09 21:17:14, robertshield wrote:
> > > Why is this #if here?
> > 
> > This code is specific to AURA. Will remove it if you feel strongly about it.
> 
> It's ok to leave it, but please add a brief comment about what it does.  

Done.

https://codereview.chromium.org/11096013/diff/3004/chrome/browser/download/do...
File chrome/browser/download/download_status_updater_win.cc (right):

https://codereview.chromium.org/11096013/diff/3004/chrome/browser/download/do...
chrome/browser/download/download_status_updater_win.cc:133:
download->GetFullPath().value().c_str());
On 2012/10/09 21:34:15, asanka wrote:
> Nit: download->GetTargetFilePath() is preferred.
> 
> For a completed download, GetFullPath() and GetTargetFilePath() is the same,
but
> the canonical method is GetTargetFilePath().

Done.

https://codereview.chromium.org/11096013/diff/12003/chrome/browser/download/d...
File chrome/browser/download/download_status_updater_win.cc (right):

https://codereview.chromium.org/11096013/diff/12003/chrome/browser/download/d...
chrome/browser/download/download_status_updater_win.cc:133:
download->GetTargetFilePath().value().c_str());
On 2012/10/09 21:46:49, cpu wrote:
> seems you are passing a pointer to a temporary string on the stack.

This is fine as the export in metro_driver copies the strings to STL strings
before operating on them.

[jar (doing other things), 2012-10-09 22:25:23]

base in patch set 7 LGTM (minimal... and mostly rubber stamp).

[cpu_(ooo_6.6-7.5), 2012-10-09 22:31:37]

lgtm

[sky, 2012-10-10 00:08:14]

LGTM