[DevTools] Add minimum size to WebInspector.View.

[DevTools] Add minimum size to WebInspector.View.

Now each View knows about it's minimum size and containers use it to layout.
Therefore split view constraints have moved from split view to the child views.

Root view now clips the content when available window size is less than minimum size of content.

BUG=none

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169963

[dgozman, 2014-03-13 17:54:40]

Could you please take a look?

[pfeldman, 2014-03-14 06:23:10]

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Au...
File Source/devtools/front_end/AuditLauncherView.js (right):

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Au...
Source/devtools/front_end/AuditLauncherView.js:39: this.setMinimumSize(new
Size(100, 50));
You could have created Size inside the method.

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/In...
File Source/devtools/front_end/InspectedPagePlaceholder.js (right):

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/In...
Source/devtools/front_end/InspectedPagePlaceholder.js:86: var totalWidth =
document.body.offsetWidth;
You change this code in every CL :)

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/So...
File Source/devtools/front_end/SourcesPanel.js (right):

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/So...
Source/devtools/front_end/SourcesPanel.js:98:
this._navigator.view.setMinimumSize(Preferences.minSidebarWidth);
Why did this compile?

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Sp...
File Source/devtools/front_end/SplitView.js (left):

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Sp...
Source/devtools/front_end/SplitView.js:562: return -1;
This -1 was used for the case when _applyConstraints was called prior to adding
component to the DOM. Not sure it is used now.

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Vi...
File Source/devtools/front_end/View.js (right):

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Vi...
Source/devtools/front_end/View.js:541: if (!this._isShowing) {
Are you sure it needs caching? Not caching would make things so much simpler.
Also, invalidation should happen on its own upon changes in the view hierarchy,
no?

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Vi...
Source/devtools/front_end/View.js:633: if (childViews[i]._parentView === this &&
childViews[i]._visible)
there is a _callOnVisibleChildren that you could use.

[dgozman, 2014-03-14 15:55:15]

PTAL

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Au...
File Source/devtools/front_end/AuditLauncherView.js (right):

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Au...
Source/devtools/front_end/AuditLauncherView.js:39: this.setMinimumSize(new
Size(100, 50));
On 2014/03/14 06:23:10, pfeldman wrote:
> You could have created Size inside the method.

Done.

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/In...
File Source/devtools/front_end/InspectedPagePlaceholder.js (right):

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/In...
Source/devtools/front_end/InspectedPagePlaceholder.js:86: var totalWidth =
document.body.offsetWidth;
On 2014/03/14 06:23:10, pfeldman wrote:
> You change this code in every CL :)

Now we have improved system on Mac, and I have more confidence in our layout
logic. We may also (theoretically) break |preferredSize >= actualSize|
invariant.

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/So...
File Source/devtools/front_end/SourcesPanel.js (right):

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/So...
Source/devtools/front_end/SourcesPanel.js:98:
this._navigator.view.setMinimumSize(Preferences.minSidebarWidth);
On 2014/03/14 06:23:10, pfeldman wrote:
> Why did this compile?

No idea.

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Sp...
File Source/devtools/front_end/SplitView.js (left):

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Sp...
Source/devtools/front_end/SplitView.js:562: return -1;
On 2014/03/14 06:23:10, pfeldman wrote:
> This -1 was used for the case when _applyConstraints was called prior to
adding
> component to the DOM. Not sure it is used now.

I've removed check for this in _innerSetSidebarSize.

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Vi...
File Source/devtools/front_end/View.js (right):

https://codereview.chromium.org/197823010/diff/1/Source/devtools/front_end/Vi...
Source/devtools/front_end/View.js:633: if (childViews[i]._parentView === this &&
childViews[i]._visible)
On 2014/03/14 06:23:10, pfeldman wrote:
> there is a _callOnVisibleChildren that you could use.

Done.

[vsevik, 2014-03-19 16:59:48]

I did not look carefully yet, but here is some comments so far.

https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
File Source/devtools/front_end/SourcesPanel.js (right):

https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
Source/devtools/front_end/SourcesPanel.js:212:
WebInspector.SourcesPanel.minToolbarWidth = 185;
This is important to keep the old value because we have a run snippet button
which not always shown.

https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
File Source/devtools/front_end/View.js (right):

https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
Source/devtools/front_end/View.js:627: calculateMinimumSize: function()
We should  look at box.minimumSize as well.
This is also true for other contaners

[dgozman, 2014-03-19 19:43:48]

https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
File Source/devtools/front_end/SourcesPanel.js (right):

https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
Source/devtools/front_end/SourcesPanel.js:212:
WebInspector.SourcesPanel.minToolbarWidth = 185;
On 2014/03/19 16:59:49, vsevik wrote:
> This is important to keep the old value because we have a run snippet button
> which not always shown.

We decided with Pavel that it's not worth it. Extra button is just clipped now,
not messing with layout.

https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
File Source/devtools/front_end/View.js (right):

https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
Source/devtools/front_end/View.js:627: calculateMinimumSize: function()
On 2014/03/19 16:59:49, vsevik wrote:
> We should  look at box.minimumSize as well.
> This is also true for other contaners

I'm not sure I understand this comment. We don't have WebInspector.Box at all.
SplitView and TabbedPane override this method as well as VBox and HBox. What's
your concern?

[vsevik, 2014-03-20 16:21:45]

On 2014/03/19 19:43:48, dgozman wrote:
>
https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
> File Source/devtools/front_end/SourcesPanel.js (right):
> 
>
https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
> Source/devtools/front_end/SourcesPanel.js:212:
> WebInspector.SourcesPanel.minToolbarWidth = 185;
> On 2014/03/19 16:59:49, vsevik wrote:
> > This is important to keep the old value because we have a run snippet button
> > which not always shown.
> 
> We decided with Pavel that it's not worth it. Extra button is just clipped
now,
> not messing with layout.
Let's change this size in a separate patch. Consider using setMinimumSize
depending on whther this button is shown.

> 
>
https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
> File Source/devtools/front_end/View.js (right):
> 
>
https://chromiumcodereview.appspot.com/197823010/diff/140001/Source/devtools/...
> Source/devtools/front_end/View.js:627: calculateMinimumSize: function()
> On 2014/03/19 16:59:49, vsevik wrote:
> > We should  look at box.minimumSize as well.
> > This is also true for other contaners
> 
> I'm not sure I understand this comment. We don't have WebInspector.Box at all.
> SplitView and TabbedPane override this method as well as VBox and HBox. What's
> your concern?
Ignore this. :)

[vsevik, 2014-03-25 12:30:51]

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
File Source/devtools/front_end/InspectorView.js (right):

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
Source/devtools/front_end/InspectorView.js:453: body.style.width = "100%";
Let's move this to css.

[vsevik, 2014-03-25 13:26:04]

lgtm

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
File Source/devtools/front_end/InspectorView.js (right):

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
Source/devtools/front_end/InspectorView.js:474: this.element.style.bottom =
bottom + "px";
Let's set scroll listener when the size is not enough only, so that we could me
sure that this is something that is done very rarely.

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
File Source/devtools/front_end/SplitView.js (right):

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
Source/devtools/front_end/SplitView.js:578: if (this._isVertical)
Does it mean we should also invalidateMinimumSize in setVertical?

[dgozman, 2014-03-25 14:13:57]

Thank you for review!

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
File Source/devtools/front_end/InspectorView.js (right):

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
Source/devtools/front_end/InspectorView.js:453: body.style.width = "100%";
On 2014/03/25 12:30:51, vsevik wrote:
> Let's move this to css.

Done.

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
Source/devtools/front_end/InspectorView.js:474: this.element.style.bottom =
bottom + "px";
On 2014/03/25 13:26:04, vsevik wrote:
> Let's set scroll listener when the size is not enough only, so that we could
me
> sure that this is something that is done very rarely.

Done.

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
File Source/devtools/front_end/SplitView.js (right):

https://chromiumcodereview.appspot.com/197823010/diff/180001/Source/devtools/...
Source/devtools/front_end/SplitView.js:578: if (this._isVertical)
On 2014/03/25 13:26:04, vsevik wrote:
> Does it mean we should also invalidateMinimumSize in setVertical?

It's already done.

[dgozman, 2014-03-25 14:14:03]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2014-03-25 14:14:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dgozman@chromium.org/197823010/220001

[commit-bot: I haz the power, 2014-03-25 15:23:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 15:23:22]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[dgozman, 2014-03-25 16:15:58]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2014-03-25 16:16:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dgozman@chromium.org/197823010/220001

[commit-bot: I haz the power, 2014-03-25 17:16:39]

Change committed as 169963