Misc. small changes in preparation for moving popup mode edge drawing.

Misc. small changes in preparation for moving popup mode edge drawing.

* Comment/variable name changes for clarity.
* Remove code to paint toolbar top separator for glass when the toolbar is invisible.  AFAICT this never has a visible effect.
* Modify how opaque frames layout popups so the responsibility to reserve extra space lies with OpaqueBrowserFrameViewLayout, not ToolbarView.
* Pass width/height instead of right/bottom to OpaqueBrowserFrameView::FillClientEdgeRects(), which will make it a bit easier to call this from another place in the future.
* Move static const declaration to match style guide.
* Declare the location bar border color as a file-scope constant in preparation for using it in more places.

BUG=none
TEST=none
R=sky@chromium.org

Committed: https://crrev.com/dca8d578aa3ab16d5153fcd0c1220bc4e6fcc2e3
Cr-Commit-Position: refs/heads/master@{#375151}

[Peter Kasting, 2016-02-10 07:07:47]

Description was changed from

==========
Misc. small changes in preparation for moving popup mode edge drawing.

BUG=none
TEST=none
==========

to

==========
Misc. small changes in preparation for moving popup mode edge drawing.

* Comment/variable name changes for clarity.
* Remove code to paint toolbar top separator for glass when the toolbar is
invisible.  AFAICT this never has a visible effect.
* Expose kContentEdgeShadowThickness from OpaqueBrowserFrameView so ToolbarView
can use it, instead of defining a separate constant that has to stay in sync.
* Pass width/height instead of right/bottom to
OpaqueBrowserFrameView::FillClientEdgeRects(), which will make it a bit easier
to call this from another place in the future.
* Move static const declaration to match style guide.
* Declare the location bar border color as a file-scope constant in preparation
for using it in more places.

BUG=none
TEST=none
==========

[Peter Kasting, 2016-02-10 07:07:47]

pkasting@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2016-02-10 17:22:35]

https://codereview.chromium.org/1682373002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/1682373002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:782: int img_h =
bottom - img_y;
nit: const to be consistent with the others.

https://codereview.chromium.org/1682373002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:815:
canvas->FillRect(gfx::Rect(x, y + height, w + (2 * kClientEdgeThickness),
Isn't this different than the old because you've modified x on 812? And isn't
the width now bigger by 2*kClientEdgeThickness?

https://codereview.chromium.org/1682373002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.h (right):

https://codereview.chromium.org/1682373002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.h:150: int height,
nti: if you're going to go with 'w' for width then be consistent and use 'h' for
height. Or instead of 'w' use width.

[Peter Kasting, 2016-02-11 06:02:02]

Description was changed from

==========
Misc. small changes in preparation for moving popup mode edge drawing.

* Comment/variable name changes for clarity.
* Remove code to paint toolbar top separator for glass when the toolbar is
invisible.  AFAICT this never has a visible effect.
* Expose kContentEdgeShadowThickness from OpaqueBrowserFrameView so ToolbarView
can use it, instead of defining a separate constant that has to stay in sync.
* Pass width/height instead of right/bottom to
OpaqueBrowserFrameView::FillClientEdgeRects(), which will make it a bit easier
to call this from another place in the future.
* Move static const declaration to match style guide.
* Declare the location bar border color as a file-scope constant in preparation
for using it in more places.

BUG=none
TEST=none
==========

to

==========
Misc. small changes in preparation for moving popup mode edge drawing.

* Comment/variable name changes for clarity.
* Remove code to paint toolbar top separator for glass when the toolbar is
invisible.  AFAICT this never has a visible effect.
* Modify how opaque frames layout popups so the responsibility to reserve extra
space lies with OpaqueBrowserFrameViewLayout, not ToolbarView.
* Pass width/height instead of right/bottom to
OpaqueBrowserFrameView::FillClientEdgeRects(), which will make it a bit easier
to call this from another place in the future.
* Move static const declaration to match style guide.
* Declare the location bar border color as a file-scope constant in preparation
for using it in more places.

BUG=none
TEST=none
==========

[Peter Kasting, 2016-02-11 06:02:32]

PTAL.  I also reworked how popup layout works for opaque frames based on a
better understanding of Terry's in-flight Ash changes.

https://codereview.chromium.org/1682373002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/1682373002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:782: int img_h =
bottom - img_y;
On 2016/02/10 17:22:35, sky wrote:
> nit: const to be consistent with the others.

Done.

https://codereview.chromium.org/1682373002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:815:
canvas->FillRect(gfx::Rect(x, y + height, w + (2 * kClientEdgeThickness),
On 2016/02/10 17:22:35, sky wrote:
> Isn't this different than the old because you've modified x on 812? And isn't
> the width now bigger by 2*kClientEdgeThickness?

Yes, and this is intentional.  I'm still drawing over the same pixels, but I've
changed how those are drawn.  Before, we drew the side rects all the way to the
bottom and the bottom inside them.  Now, we draw the bottom rect all the way to
the sides, and the sides above it.  This is necessary for the next patch.

https://codereview.chromium.org/1682373002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.h (right):

https://codereview.chromium.org/1682373002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.h:150: int height,
On 2016/02/10 17:22:35, sky wrote:
> nti: if you're going to go with 'w' for width then be consistent and use 'h'
for
> height. Or instead of 'w' use width.

Done.

[sky, 2016-02-11 16:31:49]

LGTM

[Peter Kasting, 2016-02-12 02:19:58]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2016-02-12 02:19:58]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1682373002/#ps60001
(title: "Resync")

[commit-bot: I haz the power, 2016-02-12 02:21:00]

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

[commit-bot: I haz the power, 2016-02-12 02:39:07]

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

[commit-bot: I haz the power, 2016-02-12 02:39:08]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Peter Kasting, 2016-02-12 06:46:11]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2016-02-12 06:46:11]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1682373002/#ps80001
(title: "Fix build")

[commit-bot: I haz the power, 2016-02-12 06:46:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1682373002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1682373002/80001

[commit-bot: I haz the power, 2016-02-12 08:09:48]

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

[commit-bot: I haz the power, 2016-02-12 08:09:49]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Peter Kasting, 2016-02-12 08:19:11]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2016-02-12 08:19:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1682373002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1682373002/80001

[commit-bot: I haz the power, 2016-02-12 08:48:32]

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

[commit-bot: I haz the power, 2016-02-12 08:48:33]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Peter Kasting, 2016-02-12 09:05:34]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2016-02-12 09:05:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1682373002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1682373002/80001

[commit-bot: I haz the power, 2016-02-12 09:37:41]

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

[commit-bot: I haz the power, 2016-02-12 09:37:42]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Peter Kasting, 2016-02-12 09:42:07]

Description was changed from

==========
Misc. small changes in preparation for moving popup mode edge drawing.

* Comment/variable name changes for clarity.
* Remove code to paint toolbar top separator for glass when the toolbar is
invisible.  AFAICT this never has a visible effect.
* Modify how opaque frames layout popups so the responsibility to reserve extra
space lies with OpaqueBrowserFrameViewLayout, not ToolbarView.
* Pass width/height instead of right/bottom to
OpaqueBrowserFrameView::FillClientEdgeRects(), which will make it a bit easier
to call this from another place in the future.
* Move static const declaration to match style guide.
* Declare the location bar border color as a file-scope constant in preparation
for using it in more places.

BUG=none
TEST=none
==========

to

==========
Misc. small changes in preparation for moving popup mode edge drawing.

* Comment/variable name changes for clarity.
* Remove code to paint toolbar top separator for glass when the toolbar is
invisible.  AFAICT this never has a visible effect.
* Modify how opaque frames layout popups so the responsibility to reserve extra
space lies with OpaqueBrowserFrameViewLayout, not ToolbarView.
* Pass width/height instead of right/bottom to
OpaqueBrowserFrameView::FillClientEdgeRects(), which will make it a bit easier
to call this from another place in the future.
* Move static const declaration to match style guide.
* Declare the location bar border color as a file-scope constant in preparation
for using it in more places.

BUG=none
TEST=none
R=sky@chromium.org

Committed:
https://chromium.googlesource.com/chromium/src/+/dca8d578aa3ab16d5153fcd0c122...
==========

[Peter Kasting, 2016-02-12 09:42:08]

Committed patchset #5 (id:80001) manually as
dca8d578aa3ab16d5153fcd0c1220bc4e6fcc2e3 (presubmit successful).

[commit-bot: I haz the power, 2016-02-16 22:42:26]

Description was changed from

==========
Misc. small changes in preparation for moving popup mode edge drawing.

* Comment/variable name changes for clarity.
* Remove code to paint toolbar top separator for glass when the toolbar is
invisible.  AFAICT this never has a visible effect.
* Modify how opaque frames layout popups so the responsibility to reserve extra
space lies with OpaqueBrowserFrameViewLayout, not ToolbarView.
* Pass width/height instead of right/bottom to
OpaqueBrowserFrameView::FillClientEdgeRects(), which will make it a bit easier
to call this from another place in the future.
* Move static const declaration to match style guide.
* Declare the location bar border color as a file-scope constant in preparation
for using it in more places.

BUG=none
TEST=none
R=sky@chromium.org

Committed:
https://chromium.googlesource.com/chromium/src/+/dca8d578aa3ab16d5153fcd0c122...
==========

to

==========
Misc. small changes in preparation for moving popup mode edge drawing.

* Comment/variable name changes for clarity.
* Remove code to paint toolbar top separator for glass when the toolbar is
invisible.  AFAICT this never has a visible effect.
* Modify how opaque frames layout popups so the responsibility to reserve extra
space lies with OpaqueBrowserFrameViewLayout, not ToolbarView.
* Pass width/height instead of right/bottom to
OpaqueBrowserFrameView::FillClientEdgeRects(), which will make it a bit easier
to call this from another place in the future.
* Move static const declaration to match style guide.
* Declare the location bar border color as a file-scope constant in preparation
for using it in more places.

BUG=none
TEST=none
R=sky@chromium.org

Committed: https://crrev.com/dca8d578aa3ab16d5153fcd0c1220bc4e6fcc2e3
Cr-Commit-Position: refs/heads/master@{#375151}
==========

[commit-bot: I haz the power, 2016-02-16 22:42:27]

Patchset 5 (id:??) landed as
https://crrev.com/dca8d578aa3ab16d5153fcd0c1220bc4e6fcc2e3
Cr-Commit-Position: refs/heads/master@{#375151}