Only send SelectionChanged events when the selection has actually changed....

Only send SelectionChanged events when the selection has actually changed.

Webkit's giving us too many respondToChangedSelection calls, so we keep claiming the X clipboard which messes with selection in other programs and the omnibox.

http://crbug.com/11956


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=16321

[Evan Stade, 2009-05-18 16:59:43]

So if you navigate to facebook.com, the selection gets set to the email field
(for me the selection is "Email" because I've never logged in with chrome). We
get an infinite stream of respondToChangedSelection calls.

I'm assuming it's something that fbook is doing that's causing this, although it
doesn't seem like it should be possible (i.e. I should file a bug with webkit
and the idea of this patch should be upstreamed). But perhaps this is known,
expected, and desired behavior from webkit. Does anyone happen to know?

[Evan Stade, 2009-05-18 17:03:35]

(fixing reviewers)

[darin (slow to review), 2009-05-18 17:09:28]

LGTM

http://codereview.chromium.org/115468/diff/1/3
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/115468/diff/1/3#newcode2496
Line 2496: std::string this_selection =
minor nit:  "const std::string& this_selection ="

[Evan Stade, 2009-05-18 17:14:23]

http://codereview.chromium.org/115468/diff/1/3
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/115468/diff/1/3#newcode2496
Line 2496: std::string this_selection =
On 2009/05/18 17:09:28, darin wrote:
> minor nit:  "const std::string& this_selection ="

I had that at first, but GetSelection() returns a string (not string&), so isn't
it pointless?

[darin (slow to review), 2009-05-18 18:29:09]

It just avoids a temporary string copy, perhaps only in debug builds.  That's
why I wrote minor nit ;-)