Add a proper shadow to the experimental app list search box.

Add a proper shadow to the experimental app list search box.

This CL adds a shadow border to the experimental app list's search box
view. Several changes were necessary to make this happen.

- The search box's actual content is now inside a container view so that
the shadow border will paint in the correct order.
- A custom event targeter has been placed on the search box widget to
allow clicks on the shadow area to pass through to the content beneath.
- The search box widget's position has to account for the extra space
due to the shadow border. The search box is still manipulated in terms
of its main content area and the border offsets are calculated and
applied when the widget's bounds are set.

BUG=453653
Committed: https://crrev.com/1810cd74f7f74270731e4caedd9d1739e83d34d0
Cr-Commit-Position: refs/heads/master@{#314511}

[calamity, 2015-02-03 04:31:53]

Patchset #1 (id:1) has been deleted

[calamity, 2015-02-03 04:32:08]

calamity@chromium.org changed reviewers:
+ mgiuca@chromium.org

[Matt Giuca, 2015-02-03 07:29:22]

mgiuca@chromium.org changed reviewers:
+ tapted@chromium.org

[Matt Giuca, 2015-02-03 07:29:23]

Please post before+after screenshots on the bug.

lgtm but please wait for Trent to comment on the app_list_view.cc change.

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/app_li...
File ui/app_list/views/app_list_view.cc (right):

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/app_li...
ui/app_list/views/app_list_view.cc:450: #if defined(USE_AURA)
Adding Trent to get opinion about this (as the resident Views-but-not-Aura
expert).

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/conten...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:475: search_box_bounds)));
Would have looked funnier as a giant nested expression of 6 nested function
calls.

[tapted, 2015-02-03 23:56:31]

thanks for looping me in. I'll probably need to figure out a story on Mac after
this.... not sure what this is yet. don't let it hold you up, but it it would be
nice to avoid a compile regression. I'll patch this in to do a spot check on the
compile.

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/app_li...
File ui/app_list/views/app_list_view.cc (right):

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/app_li...
ui/app_list/views/app_list_view.cc:114: class SearchBoxWindowTargeter : public
wm::MaskedWindowTargeter {
this needs to be in USE_AURA as well. And wm::MaskedWindowTargeter should have a
#include for iwyu  (also in USE_AURA) (it's currently pulled in via
bubble_window_targeter)

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/app_li...
ui/app_list/views/app_list_view.cc:117: : wm::MaskedWindowTargeter(window),
content_view_(content_view) {}
nit: can you pass content_view->GetWidget()->GetNativeWindow() to the superclass
constructor here? That would be consistent with what BubbleWindowTargeter does

content_view_->GetContentsBounds() is probably a constant too.... you could have
a gfx::Rect member instead of a raw pointer, but that feels more fragile, so
ignore this second bit.

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/app_li...
ui/app_list/views/app_list_view.cc:123:
mask->addRect(gfx::RectToSkRect(content_view_->ConvertRectToWidget(
Q: Is the ConvertRectToWidget bit needed (I only ask because
BubbleWindowTargeter doesn't use it and I'm too lazy to figure out which is more
correct).

https://codereview.chromium.org/894203004/diff/40001/ui/app_list/views/search...
File ui/app_list/views/search_box_view.cc (right):

https://codereview.chromium.org/894203004/diff/40001/ui/app_list/views/search...
ui/app_list/views/search_box_view.cc:84: SetLayoutManager(new
views::FillLayout());
nit: no (), for consistency

https://codereview.chromium.org/894203004/diff/40001/ui/app_list/views/search...
File ui/app_list/views/search_box_view.h (right):

https://codereview.chromium.org/894203004/diff/40001/ui/app_list/views/search...
ui/app_list/views/search_box_view.h:53: gfx::Rect
GetViewBoundsForSearchBoxContentsBounds(const gfx::Rect& rect);
nit: declare const?

[calamity, 2015-02-04 03:37:02]

Had to add ui/wm/core as DEPS.

+sadrul for DEPS OWNERS.

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/app_li...
File ui/app_list/views/app_list_view.cc (right):

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/app_li...
ui/app_list/views/app_list_view.cc:114: class SearchBoxWindowTargeter : public
wm::MaskedWindowTargeter {
On 2015/02/03 23:56:31, tapted wrote:
> this needs to be in USE_AURA as well. And wm::MaskedWindowTargeter should have
a
> #include for iwyu  (also in USE_AURA) (it's currently pulled in via
> bubble_window_targeter)

Done.

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/app_li...
ui/app_list/views/app_list_view.cc:117: : wm::MaskedWindowTargeter(window),
content_view_(content_view) {}
On 2015/02/03 23:56:31, tapted wrote:
> nit: can you pass content_view->GetWidget()->GetNativeWindow() to the
superclass
> constructor here? That would be consistent with what BubbleWindowTargeter does
> 
Done.
> content_view_->GetContentsBounds() is probably a constant too.... you could
have
> a gfx::Rect member instead of a raw pointer, but that feels more fragile, so
> ignore this second bit.

Actually it isn't. The search box animates, changing its contents bounds
constantly.

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/app_li...
ui/app_list/views/app_list_view.cc:123:
mask->addRect(gfx::RectToSkRect(content_view_->ConvertRectToWidget(
On 2015/02/03 23:56:31, tapted wrote:
> Q: Is the ConvertRectToWidget bit needed (I only ask because
> BubbleWindowTargeter doesn't use it and I'm too lazy to figure out which is
more
> correct).

Yeah, it can be removed.

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/conten...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/894203004/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:475: search_box_bounds)));
On 2015/02/03 07:29:23, Matt Giuca wrote:
> Would have looked funnier as a giant nested expression of 6 nested function
> calls.

Acknowledged.

https://codereview.chromium.org/894203004/diff/40001/ui/app_list/views/search...
File ui/app_list/views/search_box_view.cc (right):

https://codereview.chromium.org/894203004/diff/40001/ui/app_list/views/search...
ui/app_list/views/search_box_view.cc:84: SetLayoutManager(new
views::FillLayout());
On 2015/02/03 23:56:31, tapted wrote:
> nit: no (), for consistency

Done.

https://codereview.chromium.org/894203004/diff/40001/ui/app_list/views/search...
File ui/app_list/views/search_box_view.h (right):

https://codereview.chromium.org/894203004/diff/40001/ui/app_list/views/search...
ui/app_list/views/search_box_view.h:53: gfx::Rect
GetViewBoundsForSearchBoxContentsBounds(const gfx::Rect& rect);
On 2015/02/03 23:56:31, tapted wrote:
> nit: declare const?

Done.

[tapted, 2015-02-04 04:44:21]

lgtm

[calamity, 2015-02-04 05:05:32]

calamity@chromium.org changed reviewers:
+ sadrul@chromium.org

[calamity, 2015-02-04 05:05:32]

Actually add sadrul@ for ui/wm/core DEPS owners.

[sadrul, 2015-02-04 05:49:07]

On 2015/02/04 05:05:32, calamity wrote:
> Actually add sadrul@ for ui/wm/core DEPS owners.

lgtm

[calamity, 2015-02-04 05:54:51]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2015-02-04 05:55:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/894203004/60001

[commit-bot: I haz the power, 2015-02-04 06:35:58]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-02-04 06:36:46]

Patchset 3 (id:??) landed as
https://crrev.com/1810cd74f7f74270731e4caedd9d1739e83d34d0
Cr-Commit-Position: refs/heads/master@{#314511}