Display a banner above each panel that contains managed options....

Display a banner above each panel that contains managed options.

BUG=52099
TEST=Test with managed options and --enable-tabbed-options.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=56597

[csilv, 2010-08-14 02:02:04]

+zel, dhg for review.

Waiting for trybots, but figured I'd put this in the queue.  thx.

[csilv, 2010-08-16 22:08:02]

Internal folks can view a screenshot of the banner here:
http://www/~csilv/managed_banner.png

[csilv, 2010-08-16 22:10:07]

FYI, the banner is patterned after the existing banner that exists in the native
options window on Mac.  Same gradient, icon, and text string.

[zel, 2010-08-17 20:51:24]

http://codereview.chromium.org/3108018/diff/38001/39001
File chrome/browser/dom_ui/advanced_options_handler.cc (right):

http://codereview.chromium.org/3108018/diff/38001/39001#newcode133
chrome/browser/dom_ui/advanced_options_handler.cc:133: banner_handler_.reset(
can we avoid having more than one instance of OptionsManagedBannerHandler?

http://codereview.chromium.org/3108018/diff/38001/39006
File chrome/browser/dom_ui/options_managed_banner_handler.cc (right):

http://codereview.chromium.org/3108018/diff/38001/39006#newcode28
chrome/browser/dom_ui/options_managed_banner_handler.cc:28: switch (page_) {
is this the only place where you use page_?

If so, why don't you replace it with page_name_ so this class won't longer need
to know where it is being used.

[csilv, 2010-08-18 01:17:54]

http://codereview.chromium.org/3108018/diff/38001/39001
File chrome/browser/dom_ui/advanced_options_handler.cc (right):

http://codereview.chromium.org/3108018/diff/38001/39001#newcode133
chrome/browser/dom_ui/advanced_options_handler.cc:133: banner_handler_.reset(
On 2010/08/17 20:51:24, zel wrote:
> can we avoid having more than one instance of OptionsManagedBannerHandler?

OptionsManagedBannerHandler is a subclass of ManagedPrefsBannerBase which is an
existing class used by the native option windows.  ManagedPrefsBannerBase works
for a single panel only, specified as the 2nd argument in the constructor.  If
we wanted to have a single instance, we'll need to create it each time the user
changes prefs panels, but we don't currently have a mechanism to notify the
handler when the panel is about to be displayed.

On the plus side, OptionsManagedBannerHandler is fairly light-weight.  But if
you feel this is still a problem, I can add a message when the panel changes so
each panel can create this on-the-fly.

http://codereview.chromium.org/3108018/diff/38001/39006
File chrome/browser/dom_ui/options_managed_banner_handler.cc (right):

http://codereview.chromium.org/3108018/diff/38001/39006#newcode28
chrome/browser/dom_ui/options_managed_banner_handler.cc:28: switch (page_) {
On 2010/08/17 20:51:24, zel wrote:
> is this the only place where you use page_?
> 
> If so, why don't you replace it with page_name_ so this class won't longer
need
> to know where it is being used. 

The page is needed by the base class ManagedPrefsBannerBase so I can't entirely
get rid of it, but your suggestion of passing the string name in makes sense, so
I went ahead and applied that change.

[zel, 2010-08-18 20:23:26]

LGTM