[Mac] Give page actions a default image when preview enabled.

[Mac] Give page actions a default image when preview enabled.

During install it is possible to display (or layout) the action before
the images have come in.  Provide a default image for this case.

BUG=50771
TEST=See bug.

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

[rohitrao (ping after 24h), 2010-07-30 20:20:01]

LGTM

This is just going to be a blank icon?  Does the real image come in
when the bubble pops open, or do we end up with a bubble pointing at
nothing?

On Fri, Jul 30, 2010 at 4:14 PM,  <shess@chromium.org> wrote:
> Reviewers: rohitrao,
>
> Description:
> [Mac] Give page actions a default image when preview enabled.
>
> During install it is possible to display (or layout) the action before
> the images have come in.  Provide a default image for this case.
>
> BUG=50771
> TEST=See bug.
>
> Please review this at http://codereview.chromium.org/3047031/show
>
> SVN Base: git://codf21.jail/chromium.git
>
> Affected files:
>  M chrome/browser/cocoa/location_bar/page_action_decoration.mm
>
>
> Index: chrome/browser/cocoa/location_bar/page_action_decoration.mm
> diff --git a/chrome/browser/cocoa/location_bar/page_action_decoration.mm
> b/chrome/browser/cocoa/location_bar/page_action_decoration.mm
> index
>
1813bb858df5f16851465c283e73a971f5c20ac9..807040c2339a489d7a94dc5854f368971f68abb0
> 100644
> --- a/chrome/browser/cocoa/location_bar/page_action_decoration.mm
> +++ b/chrome/browser/cocoa/location_bar/page_action_decoration.mm
> @@ -141,8 +141,16 @@ void PageActionDecoration::UpdateVisibility(
>       }
>     }
>
> -    if (!skia_icon.isNull())
> +    if (!skia_icon.isNull()) {
>       SetImage(gfx::SkBitmapToNSImage(skia_icon));
> +    } else if (!GetImage()) {
> +      // During install the action can be displayed before the icons
> +      // have come in.  Rather than deal with this in multiple places,
> +      // provide a placeholder image.
> +      const NSSize default_size =
> NSMakeSize(Extension::kPageActionIconMaxSize,
> +
> Extension::kPageActionIconMaxSize);
> +      SetImage([[NSImage alloc] initWithSize:default_size]);
> +    }
>   }
>   if (IsVisible() != visible) {
>     SetVisible(visible);
>
>
>

[Scott Hess - ex-Googler, 2010-07-30 20:39:54]

On 2010/07/30 20:20:01, rohitrao wrote:
> This is just going to be a blank icon?  Does the real image come in
> when the bubble pops open, or do we end up with a bubble pointing at
> nothing?

If no icon ever comes in, yes, would point at nothing.  Am willing to leave that
for another day.

When an icon comes in, OnImageLoaded() is called, which bounces to location-bar
to UpdatePageAction(), which will eventually call UpdateVisibility() for all
page actions (which will load the new icon) and Layout() to put everything in
place.  Does that sound round-a-bout and over-kill?  Yeah.  Seems like it could
just set the icon and call location-bar for refresh (with a flag for "my size
changed" to make things efficient).  But maybe there's a good reason for the
current structure.

Thanks,
scott