DevTools: remove unused resizing strategy code paths, make it solely bounds-based.

DevTools: remove unused resizing strategy code paths, make it solely bounds-based; support hidden inspected contents case.

TBR=tsepez (removing method from devtools dispatcher)
NOTRY=true

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

[dgozman, 2014-08-04 15:31:19]

devtools lgtm

[pfeldman, 2014-08-04 15:50:28]

+ OWNERS:

sky@: ui/views
avi@: ui/cocoa

please!

[sky, 2014-08-04 17:01:25]

https://codereview.chromium.org/440663003/diff/100001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/440663003/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_view.cc:2138:
contents_container_->ReorderChildView(contents_web_view_, order);
Why do you need the reoder here?

[pfeldman, 2014-08-04 17:10:30]

On 2014/08/04 17:01:25, sky wrote:
>
https://codereview.chromium.org/440663003/diff/100001/chrome/browser/ui/views...
> File chrome/browser/ui/views/frame/browser_view.cc (right):
> 
>
https://codereview.chromium.org/440663003/diff/100001/chrome/browser/ui/views...
> chrome/browser/ui/views/frame/browser_view.cc:2138:
> contents_container_->ReorderChildView(contents_web_view_, order);
> Why do you need the reoder here?

I tried calling SetVisible(false) on contents_web_view_ , but I was not getting
the input events unless I reordered the layers. When I did reorder, SetVisible
was no longer necessary.

[Avi (use Gerrit), 2014-08-04 17:59:59]

I'm not an expert here. This looks plausible. LGTM

[sky, 2014-08-04 20:16:09]

I'm still confused here. What does the stacking order look like with and without
your patch? What should it look like?

[pfeldman, 2014-08-04 20:43:30]

When devtools is docked, container has two visible children: devtools contents
and inspected web page contents. Devtools contents is below, taking entire
screen. Web page contents is above, only taking portion of the screen. Clicking
in web page's area hits inspected page, outside of it hits devtools.

Before the patch it was always the case. My patch introduces a mode where
inspected page contents can be entirely hidden while devtools takes all the
container's space. So I pop devtools on top (it already is screen wide) and
hence the inspected page gets totally hidden.

This new mode is going to be used when we tilt the layers tree, visualize paint
profile, etc. We effectively hide the inspected page and show something else on
its place.

[sky, 2014-08-04 21:47:38]

Could you explicitly reorder the view you care about then? That is,
get the index of the view you want things relative to then restack the
target above/below that? This is more code, but makes things clearer.
Also, add a comment (which would have saved this discussion).

On Mon, Aug 4, 2014 at 1:43 PM,  <pfeldman@chromium.org> wrote:
> When devtools is docked, container has two visible children: devtools
> contents
> and inspected web page contents. Devtools contents is below, taking entire
> screen. Web page contents is above, only taking portion of the screen.
> Clicking
> in web page's area hits inspected page, outside of it hits devtools.
>
> Before the patch it was always the case. My patch introduces a mode where
> inspected page contents can be entirely hidden while devtools takes all the
> container's space. So I pop devtools on top (it already is screen wide) and
> hence the inspected page gets totally hidden.
>
> This new mode is going to be used when we tilt the layers tree, visualize
> paint
> profile, etc. We effectively hide the inspected page and show something else
> on
> its place.
>
> https://codereview.chromium.org/440663003/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[pfeldman, 2014-08-05 08:10:49]

On 2014/08/04 21:47:38, sky wrote:
> Could you explicitly reorder the view you care about then? That is,
> get the index of the view you want things relative to then restack the
> target above/below that? This is more code, but makes things clearer.
> Also, add a comment (which would have saved this discussion).

Sure I can do that, note though that there are only two views in this container
and all I care about is having one of them on top or not.

[pfeldman, 2014-08-05 08:22:54]

PTAL

[sky, 2014-08-05 16:27:48]

LGTM

https://codereview.chromium.org/440663003/diff/120001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/440663003/diff/120001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_view.cc:2144: bool devtoolsIsOnTop =
devtools_index > contents_index;
devtools_is_on_top.

[pfeldman, 2014-08-05 19:17:33]

> chrome/browser/ui/views/frame/browser_view.cc:2144: bool devtoolsIsOnTop =
> devtools_index > contents_index;
> devtools_is_on_top.

ouch.

[pfeldman, 2014-08-07 09:17:11]

The CQ bit was checked by pfeldman@chromium.org

[commit-bot: I haz the power, 2014-08-07 09:18:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pfeldman@chromium.org/440663003/140001

[pfeldman, 2014-08-07 14:07:00]

The CQ bit was checked by pfeldman@chromium.org

[commit-bot: I haz the power, 2014-08-07 14:07:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pfeldman@chromium.org/440663003/140001

[commit-bot: I haz the power, 2014-08-07 15:54:01]

Change committed as 288056