views: Make View::set_border() take a scoped_ptr<>.

ui/views/view.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef UI_VIEWS_VIEW_H_
 #define UI_VIEWS_VIEW_H_
 
 #include <algorithm>
 #include <map>
 #include <set>
@@ skipping 495 matching lines @@
   // to paint itself via the various OnPaint*() event handlers and then paints
   // the hierarchy beneath it.
   virtual void Paint(gfx::Canvas* canvas);
 
   // The background object is owned by this object and may be NULL.
   void set_background(Background* b);
   const Background* background() const { return background_.get(); }
   Background* background() { return background_.get(); }
 
   // The border object is owned by this object and may be NULL.
-  void set_border(Border* b);
+  void SetBorder(scoped_ptr<Border> b);
+  void ClearBorder();

[sky, 2014/01/23 15:30:54]

Why do we need ClearBorder? I get that it saves some code, but from an API
standpoint I don't like it. I prefer callers use SetBorder with an empty
scopedptr.

[Elliot Glaysher, 2014/01/23 18:55:26]

Because it's really visually ugly and not clear at a glance.

It was a common pattern in our code to do "set_border(NULL);" to clear a border.
That's not great semantically, but is still tolerable.

"SetBorder(scoped_ptr<views::Border>());" isn't immediately clear like NULL is,
and it's generally better to have function names document what they're doing (in
this case, we're almost always clearing a border set by our base class).

[sky, 2014/01/23 19:39:36]

That argument applies anywhere we take a scoped_ptr now. To my knowledge we are
not adding explicit 'clear' methods every where.
I'm still against ClearBorder. I don't like bloating the API.

   const Border* border() const { return border_.get(); }
   Border* border() { return border_.get(); }
 
   // Get the theme provider from the parent widget.
   virtual ui::ThemeProvider* GetThemeProvider() const;
 
   // Returns the NativeTheme to use for this View. This calls through to
   // GetNativeTheme() on the Widget this View is in. If this View is not in a
   // Widget this returns ui::NativeTheme::instance().
   ui::NativeTheme* GetNativeTheme() {
@@ skipping 1033 matching lines @@
   // Belongs to this view, but it's reference-counted on some platforms
   // so we can't use a scoped_ptr. It's dereferenced in the destructor.
   NativeViewAccessibility* native_view_accessibility_;
 
   DISALLOW_COPY_AND_ASSIGN(View);
 };
 
 }  // namespace views
 
 #endif  // UI_VIEWS_VIEW_H_