Move ImageWindowDelegate to ui/aura_extra

ui/aura_extra/image_window_delegate.cc

Index: ui/aura_extra/image_window_delegate.cc
diff --git a/content/browser/web_contents/aura/image_window_delegate.cc b/ui/aura_extra/image_window_delegate.cc
similarity index 73%
rename from content/browser/web_contents/aura/image_window_delegate.cc
rename to ui/aura_extra/image_window_delegate.cc
index 25878ea2c1c7bac602238f13a5cdd3507d1c7abb..6663ecaea0fa7eca2b7f306807b11d0e86f2508c 100644
--- a/content/browser/web_contents/aura/image_window_delegate.cc
+++ b/ui/aura_extra/image_window_delegate.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/browser/web_contents/aura/image_window_delegate.h"
+#include "ui/aura_extra/image_window_delegate.h"
 
 #include "ui/base/cursor/cursor.h"
 #include "ui/base/hit_test.h"
@@ -13,21 +13,35 @@
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
 
-namespace content {
+namespace aura_extra {
 
 ImageWindowDelegate::ImageWindowDelegate()
-    : size_mismatch_(false) {
+    : background_color_(SK_ColorWHITE),
+      self_destroy_(true),
+      size_mismatch_(false) {
 }
 
 ImageWindowDelegate::~ImageWindowDelegate() {
 }
 
+void ImageWindowDelegate::SetBackgroundColor(SkColor color) {
+  background_color_ = color;
+}
+
 void ImageWindowDelegate::SetImage(const gfx::Image& image) {
   image_ = image;
   if (!window_size_.IsEmpty() && !image_.IsEmpty())
     size_mismatch_ = window_size_ != image_.AsImageSkia().size();
 }
 
+void ImageWindowDelegate::SetImageOffset(gfx::Vector2d offset) {
+  offset_ = offset;
+}
+
+void ImageWindowDelegate::SetSelfDestroy(bool self_destroy) {
+  self_destroy_ = self_destroy;

[sadrul, 2015/01/28 18:26:56]

Is this really necessary? It looks like this is used by TouchHandleDrawableAura,
but it could simply override OnWindowDestroyed instead to not 'delete this;'

[mohsen, 2015/01/28 21:10:29]

Actually, that was Mikhail's original approach, but, I found it a bit confusing
as an ImageWindowDelegate is expected to self-destroy, while
TouchHandleDrawableAura, which is an ImageWindowDelegate, does not self-destroy.

Do you think I should switch back to that approach?

[sadrul, 2015/01/28 21:13:39]

Yeah, I think that'd be cleaner. (some other parts of the code base does this,
e.g. RenderViewObserver, BrowserMessageFilter etc.) Make sure to explain this
clearly in ImageWindowDelegate doc

[mohsen, 2015/01/29 03:23:15]

Done.

+}
+
 gfx::Size ImageWindowDelegate::GetMinimumSize() const {
   return gfx::Size();
 }
@@ -65,13 +79,12 @@ void ImageWindowDelegate::OnCaptureLost() {
 }
 
 void ImageWindowDelegate::OnPaint(gfx::Canvas* canvas) {
-  if (image_.IsEmpty()) {
-    canvas->DrawColor(SK_ColorWHITE);
-  } else {
-    if (size_mismatch_)
-      canvas->DrawColor(SK_ColorWHITE);
-    canvas->DrawImageInt(image_.AsImageSkia(), 0, 0);
+  if (background_color_ != SK_ColorTRANSPARENT &&
+      (image_.IsEmpty() || size_mismatch_ || !offset_.IsZero())) {
+    canvas->DrawColor(background_color_);
   }
+  if (!image_.IsEmpty())
+    canvas->DrawImageInt(image_.AsImageSkia(), offset_.x(), offset_.y());
 }

[sadrul, 2015/01/28 18:26:56]

Can this be:

  if (image_.IsEmpty() || size_mismatch_)
    DrawColor

  if (!image_.IsEmpty())
    DrawImage

(whether we should paint an image or a color shouldn't depend on what the
background color is, or what the image offset is).

[mfomitchev, 2015/01/28 19:15:42]

If offset is non-zero we should paint the background color even if the sizes
match..

If the background color is SK_ColorTRANSPARENT, then there's no reason to paint
it (although there's no harm in doing so other than potential waste of cpu).

[sadrul, 2015/01/28 19:28:47]

Why?

[sadrul, 2015/01/28 19:30:04]

Oh, I see, because there will be parts of the Window that will remain unpainted
otherwise. OK, that makes sense.

 
 void ImageWindowDelegate::OnDeviceScaleFactorChanged(float scale_factor) {
@@ -81,7 +94,8 @@ void ImageWindowDelegate::OnWindowDestroying(aura::Window* window) {
 }
 
 void ImageWindowDelegate::OnWindowDestroyed(aura::Window* window) {
-  delete this;
+  if (self_destroy_)
+    delete this;
 }
 
 void ImageWindowDelegate::OnWindowTargetVisibilityChanged(bool visible) {
@@ -94,4 +108,4 @@ bool ImageWindowDelegate::HasHitTestMask() const {
 void ImageWindowDelegate::GetHitTestMask(gfx::Path* mask) const {
 }
 
-}  // namespace content
+}  // namespace aura_extra