views::Textfield: Implement yank editing command.

ui/views/views_delegate.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_VIEWS_DELEGATE_H_
 #define UI_VIEWS_VIEWS_DELEGATE_H_
 
 #include <string>
 
 #if defined(OS_WIN)
@@ skipping 84 matching lines @@
 
   // Call this method to set a factory callback that will be used to construct
   // NativeWidget implementations overriding the platform defaults.
   void set_native_widget_factory(NativeWidgetFactory factory) {
     native_widget_factory_ = factory;
   }
   const NativeWidgetFactory& native_widget_factory() {
     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,
                                    const std::string& window_name,
                                    const gfx::Rect& bounds,
                                    ui::WindowShowState show_state);
 
   // Retrieves the saved position and size and "show" state for the window with
   // the specified name.
   virtual bool GetSavedWindowPlacement(const Widget* widget,
@@ skipping 83 matching lines @@
 
  private:
   std::unique_ptr<ViewsTouchEditingControllerFactory> views_tsc_factory_;
 
 #if defined(USE_AURA)
   std::unique_ptr<TouchSelectionMenuRunnerViews> touch_selection_menu_runner_;
 #endif
 
   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);
 };
 
 }  // namespace views
 
 #endif  // UI_VIEWS_VIEWS_DELEGATE_H_