views::Textfield: Implement yank editing command.

ui/views/views_delegate.h

Index: ui/views/views_delegate.h
diff --git a/ui/views/views_delegate.h b/ui/views/views_delegate.h
index 54b5c4383e8d39aaf32dfdc58856ff712925a194..f83b1339f8edd1bcc543c31491f89413f4fcdbf8 100644
--- a/ui/views/views_delegate.h
+++ b/ui/views/views_delegate.h
@@ -102,6 +102,12 @@ class VIEWS_EXPORT ViewsDelegate {
     return native_widget_factory_;
   }
 
+  // Access the text buffer used to implement the "Yank" text edit command.
+  void set_kill_buffer(const base::string16& kill_buffer) {

[sky, 2016/07/20 15:45:57]

Is there a reason this needs to be on ViewsDelegate and not a static
(base::string16*) in TextfieldModel? AFAICT it's only used by TextfieldModel.

[karandeepb, 2016/07/21 00:44:58]

The cpp style guide forbids static storage duration variables unless they are
constexpr, owing to thread safety issues -
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#....
If thread safety is not an issue here, I can change it to a function static
variable residing in textfield_model.cc.

Though another benefit of adding it to ViewsDelegate is that we don't need to
clear global state between different tests.

[tapted, 2016/07/21 01:08:23]

this came up in review earlier at
https://codereview.chromium.org/2119813002/diff/100001/ui/views/controls/text...

Chromium gets around the "no non-POD types" with things like
CR_DEFINE_STATIC_LOCAL, which I offered in a code snippet in that review comment
(a nullptr string16* `new`ed on first use would also work). So the style-guide
isn't really a deal-breaker, but ViewsDelegate seemed like an elegant way to
partition the sharing-between-textfields behaviour without global state leaking
between tests.

We could also do something like having the ViewsTestHelper destructor do

~ViewsTestHelper() {
  TextFieldModel::ClearKillBufferForTest();
}

(i.e. along with exposing the static ClearKillBufferForTest() method somewhere).


Depends which one sky prefers I guess :)

+    kill_buffer_ = kill_buffer;
+  }
+  const base::string16& kill_buffer() const { return kill_buffer_; }
+
   // Saves the position, size and "show" state for the window with the
   // specified name.
   virtual void SaveWindowPlacement(const Widget* widget,
@@ -205,6 +211,14 @@ class VIEWS_EXPORT ViewsDelegate {
 
   NativeWidgetFactory native_widget_factory_;
 
+  // Buffer containing the text to be inserted on executing yank command for
+  // views::Texfield. Member of ViewsDelegate since this needs to be persisted
+  // across multiple textfield instances.
+  // On Mac, the size of the kill ring (no. of buffers) is controlled by
+  // NSTextKillRingSize, a text system default. However to keep things simple,
+  // the default kill ring size of 1 (i.e. a single buffer) is assumed.
+  base::string16 kill_buffer_;
+
   DISALLOW_COPY_AND_ASSIGN(ViewsDelegate);
 };