MacViews: Add native drop shadow to dialogs on OSX < 10.10.

ui/views/view.cc

Index: ui/views/view.cc
diff --git a/ui/views/view.cc b/ui/views/view.cc
index 375cff4a2ca640a14dbf42c59b3b04411e253b28..d2bc20514cfd07eec11e2ac2e2e85437bd5731ec 100644
--- a/ui/views/view.cc
+++ b/ui/views/view.cc
@@ -798,16 +798,28 @@ void View::Paint(const ui::PaintContext& parent_context) {
   // std::optional once we can do so.
   ui::ClipRecorder clip_recorder(parent_context);
   if (paint_relative_to_parent) {
-    // Set the clip rect to the bounds of this View. Note that the X (or left)
-    // position we pass to ClipRect takes into consideration whether or not the
-    // View uses a right-to-left layout so that we paint the View in its
-    // mirrored position if need be.
-    gfx::Rect clip_rect_in_parent = bounds();
-    clip_rect_in_parent.Inset(clip_insets_);
-    if (parent_)
-      clip_rect_in_parent.set_x(
-          parent_->GetMirroredXForRect(clip_rect_in_parent));
-    clip_recorder.ClipRect(clip_rect_in_parent);
+    // If the View is a root view (has no parent) and if the window shape is

[tapted, 2016/01/28 05:59:22]

only-do-if-root-view logic probably belongs in views::RootView

E.g. you could have

class View {
..
protected:
  virtual void ApplyClip(ui::ClipRecorder* clip_recorder);
};

|clip_insets_| should be removed from views::View and put into
app_list::SearchResultPageView instead, which can override ApplyClip, as well as
views::RootView.

[karandeepb, 2016/02/04 03:39:28]

Removed the changes in view.cc.

+    // non-rectangular, retreive the window mask from the non-client view and
+    // use it as the clipping path. Else set the clip rect to the bounds of this
+    // View. Note that the X (or left) position we pass to ClipRect takes into
+    // consideration whether or not the View uses a right-to-left layout so that
+    // we paint the View in its mirrored position if need be.
+    bool clip_set = false;

[karandeepb, 2016/01/27 23:25:03]

I have added this to prevent the distortion that occurred on view like Bookmark
bubble, which paint over the window boundary, and don't have rounded corners.
Have checked this on OSX 10.10, yet to test it on windows. However I am not sure
whether painting always resumes from root view or not. If it doesn't it may
happen that the child view may still paint outside the window mask.

[tapted, 2016/01/28 05:59:22]

Does this fix the rounded bottom corners on the bookmark bubble on
Windows/ChromeOS as well as on Mac? If so, then I think this is a good approach,
and would be good to split out separately. Some graphics experts should weigh in
-- there might be performance concerns with more complex clipping paths. They
might also know a better place/method of achieving the fix.

If we are only concerned about Mac, then I think we have the option of creating
a CAShapeLayer instead of a regular CALayer for |background_layer| in
BridgedNativeWidget::AddCompositorView(..). But since this is fixing a bug on
other platforms - something here might be right.

[karandeepb, 2016/02/04 03:39:28]

This didn't fix the bug on other platforms since a rectangular mask is returned
for BubbleBorder::SMALL_SHADOW. 
On OSX 10.9, the distortion around the corners on bubbles like BookmarkBubble
isn't visible with the current border radius of 2 for BubbleBorder::NO_ASSETS,
however, it becomes visible if we increase the border radius. 

This can be fixed on Mac, by setting a cornerRadius on background_layer and
setting its masksToBounds to YES or by setting a mask layer on the background
layer which again would depend on the window mask. However, it may be slightly
computationally expensive. 
Alternatively, as you had recommended I can look into performing the clipping in
some views::subview which does not include the raster shadow.

+    if (!parent() && GetWidget()->non_client_view()) {
+      gfx::Path path;
+      GetWidget()->non_client_view()->GetWindowMask(bounds().size(), &path);
+      if (!path.isEmpty()) {
+        clip_recorder.ClipPathWithAntiAliasing(path);
+        clip_set = true;
+      }
+    } else if (!clip_set) {

[tapted, 2016/01/28 05:59:22]

I think this is equivalent to `else` -- |clip_set| can't change value before
this is checked. (or maybe this should be `if` instead of `else if`).

[karandeepb, 2016/02/04 03:39:28]

Done.

+      gfx::Rect clip_rect_in_parent = bounds();
+      clip_rect_in_parent.Inset(clip_insets_);
+      if (parent_)
+        clip_rect_in_parent.set_x(
+            parent_->GetMirroredXForRect(clip_rect_in_parent));
+      clip_recorder.ClipRect(clip_rect_in_parent);
+    }
   }
 
   ui::TransformRecorder transform_recorder(context);