Experimental app list: custom launcher pages handle web contents events.

Experimental app list: custom launcher pages handle web contents events.

CustomLauncherPageContents now implements WebContentsDelegate. Now web
features that rely on the browser, such as clicking links and opening
file choosers, are handled rather than being ignored.

BUG=391137
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287331

[Matt Giuca, 2014-08-04 00:17:54]

I think this might need a small rebasing against changes made to the previous
CL, but nothing that will block the review.

[benwells, 2014-08-04 02:35:20]

https://codereview.chromium.org/404383002/diff/60001/apps/custom_launcher_pag...
File apps/custom_launcher_page_contents.h (right):

https://codereview.chromium.org/404383002/diff/60001/apps/custom_launcher_pag...
apps/custom_launcher_page_contents.h:34: CustomLauncherPageContents(AppDelegate*
app_delegate,
If you make this a scoped_ptr<AppDelegate> it will communicate that it takes
ownership all by itself.

https://codereview.chromium.org/404383002/diff/60001/chrome/browser/ui/app_li...
File chrome/browser/ui/app_list/app_list_view_delegate.cc (right):

https://codereview.chromium.org/404383002/diff/60001/chrome/browser/ui/app_li...
chrome/browser/ui/app_list/app_list_view_delegate.cc:27: #include
"chrome/browser/ui/apps/chrome_app_window_delegate.h"
I assume this is part of the 'rebase might be needed' thing.

[Matt Giuca, 2014-08-04 06:25:58]

https://codereview.chromium.org/404383002/diff/60001/apps/custom_launcher_pag...
File apps/custom_launcher_page_contents.h (right):

https://codereview.chromium.org/404383002/diff/60001/apps/custom_launcher_pag...
apps/custom_launcher_page_contents.h:34: CustomLauncherPageContents(AppDelegate*
app_delegate,
On 2014/08/04 02:35:20, benwells wrote:
> If you make this a scoped_ptr<AppDelegate> it will communicate that it takes
> ownership all by itself.

Done.

(After discussing with Sam about how to do it --- is this what you meant? Ain't
never seen this pattern before; I've always done it and seen it done with a raw
pointer.)

(AppWindow takes a raw pointer too, which is what this code is inspired by.)

https://codereview.chromium.org/404383002/diff/60001/chrome/browser/ui/app_li...
File chrome/browser/ui/app_list/app_list_view_delegate.cc (right):

https://codereview.chromium.org/404383002/diff/60001/chrome/browser/ui/app_li...
chrome/browser/ui/app_list/app_list_view_delegate.cc:27: #include
"chrome/browser/ui/apps/chrome_app_window_delegate.h"
On 2014/08/04 02:35:20, benwells wrote:
> I assume this is part of the 'rebase might be needed' thing.

Indeed.

[benwells, 2014-08-04 06:53:16]

lgtm

https://codereview.chromium.org/404383002/diff/60001/apps/custom_launcher_pag...
File apps/custom_launcher_page_contents.h (right):

https://codereview.chromium.org/404383002/diff/60001/apps/custom_launcher_pag...
apps/custom_launcher_page_contents.h:34: CustomLauncherPageContents(AppDelegate*
app_delegate,
On 2014/08/04 06:25:58, Matt Giuca wrote:
> On 2014/08/04 02:35:20, benwells wrote:
> > If you make this a scoped_ptr<AppDelegate> it will communicate that it takes
> > ownership all by itself.
> 
> Done.
> 
> (After discussing with Sam about how to do it --- is this what you meant?
Ain't
> never seen this pattern before; I've always done it and seen it done with a
raw
> pointer.)
> 
> (AppWindow takes a raw pointer too, which is what this code is inspired by.)

Yes was what I meant. I'm not sure if you've got the Pass calls in the right
places but if it compiles, you have.

I haven't always done things this way but my understanding is it is the
preferred way of passing pointers with ownership.

[Matt Giuca, 2014-08-04 07:00:40]

> Yes was what I meant. I'm not sure if you've got the Pass calls in the right
> places but if it compiles, you have.

Yes, I believe so (confirmed independently with Sam and Chris). On the caller
side, no Pass is needed because scoped_ptr is an rvalue; it gets constructed
directly in the argument stack of the constructor and doesn't have to be copied.
On the constructor side, Pass is required to move it from its arguments stack
into the object being initialized.

> I haven't always done things this way but my understanding is it is the
> preferred way of passing pointers with ownership.

Apparently it is.

[Matt Giuca, 2014-08-04 10:41:22]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2014-08-04 10:41:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mgiuca@chromium.org/404383002/100001

[commit-bot: I haz the power, 2014-08-04 13:52:12]

Change committed as 287331