linux-aura: Restore middle-click on new-tab button behaviour.

linux-aura: Restore middle-click on new-tab button behaviour.

Middle-clicking on new-tab button opens a new tab and navigates to the
URL in the clipboard, or navigates to the search-result page for the
clipboard content if it's not a URL.

BUG=319011
R=sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=235469

[Markus (顧孟勤), 2013-11-15 19:11:29]

Overall this looks pretty good, even if it doesn't solve all of the problems
with selections (e.g. we still can't select the contents of the Omnibox and then
paste it with the middle mouse button)

Just one minor nitpick in case there is no selection ... and you can tell me
that this is working-as-intended. But if you do so, please add a comment.

[Markus (顧孟勤), 2013-11-15 19:11:53]

https://codereview.chromium.org/74133003/diff/1/chrome/browser/ui/views/tabs/...
File chrome/browser/ui/views/tabs/tab_strip.cc (right):

https://codereview.chromium.org/74133003/diff/1/chrome/browser/ui/views/tabs/...
chrome/browser/ui/views/tabs/tab_strip.cc:1532:
controller()->CreateNewTabWithLocation(clipboard_text);
Should we check whether clipboard_text is empty?

[sadrul, 2013-11-15 19:57:21]

On 2013/11/15 19:11:29, Markus (顧孟勤) wrote:
> Overall this looks pretty good, even if it doesn't solve all of the problems
> with selections (e.g. we still can't select the contents of the Omnibox and
then
> paste it with the middle mouse button)

I think msw@ is working on a patch for that
(https://codereview.chromium.org/73403002/)

> 
> Just one minor nitpick in case there is no selection ... and you can tell me
> that this is working-as-intended. But if you do so, please add a comment.

The current behaviour in linux-gtk is to not do anything if there's no
selection. This behaviour is maintained in linux-aura with this patch.

https://codereview.chromium.org/74133003/diff/1/chrome/browser/ui/views/tabs/...
File chrome/browser/ui/views/tabs/tab_strip.cc (right):

https://codereview.chromium.org/74133003/diff/1/chrome/browser/ui/views/tabs/...
chrome/browser/ui/views/tabs/tab_strip.cc:1532:
controller()->CreateNewTabWithLocation(clipboard_text);
On 2013/11/15 19:11:53, Markus (顧孟勤) wrote:
> Should we check whether clipboard_text is empty?

Done.

[sky, 2013-11-15 20:57:09]

https://codereview.chromium.org/74133003/diff/100001/chrome/browser/ui/tabs/t...
File chrome/browser/ui/tabs/test_tab_strip_model_delegate.h (right):

https://codereview.chromium.org/74133003/diff/100001/chrome/browser/ui/tabs/t...
chrome/browser/ui/tabs/test_tab_strip_model_delegate.h:20: virtual void
AddURLTabAt(const GURL& url, int index, bool foregroud) OVERRIDE;
Can you combine AddBlankTabAt and AddURLTabAt? Maybe an empty url could be
treated as blank.

[sadrul, 2013-11-15 21:46:55]

https://codereview.chromium.org/74133003/diff/100001/chrome/browser/ui/tabs/t...
File chrome/browser/ui/tabs/test_tab_strip_model_delegate.h (right):

https://codereview.chromium.org/74133003/diff/100001/chrome/browser/ui/tabs/t...
chrome/browser/ui/tabs/test_tab_strip_model_delegate.h:20: virtual void
AddURLTabAt(const GURL& url, int index, bool foregroud) OVERRIDE;
On 2013/11/15 20:57:09, sky wrote:
> Can you combine AddBlankTabAt and AddURLTabAt? Maybe an empty url could be
> treated as blank.

Done. I combined these two only in TabStripModelDelegate(). I introduced a
chrome::AddURLTabAt(), which chrome::AddBlankTabAt() uses. I will have a
follow-up patch to remove chrome::AddBlankTabAt() and replace the callsites to
use AddURLTabAt() instead. That should be a fairly straight-forward mechanical
change.

[sky, 2013-11-15 22:01:32]

LGTM

https://codereview.chromium.org/74133003/diff/210015/chrome/browser/ui/browse...
File chrome/browser/ui/browser_tab_strip_model_delegate.h (right):

https://codereview.chromium.org/74133003/diff/210015/chrome/browser/ui/browse...
chrome/browser/ui/browser_tab_strip_model_delegate.h:23: virtual void
AddURLTabAt(const GURL& u, int index, bool foreground) OVERRIDE;
nit: url (to match url in .cc), and url is more descriptive too.

[sadrul, 2013-11-15 22:41:27]

Thanks!

https://codereview.chromium.org/74133003/diff/210015/chrome/browser/ui/browse...
File chrome/browser/ui/browser_tab_strip_model_delegate.h (right):

https://codereview.chromium.org/74133003/diff/210015/chrome/browser/ui/browse...
chrome/browser/ui/browser_tab_strip_model_delegate.h:23: virtual void
AddURLTabAt(const GURL& u, int index, bool foreground) OVERRIDE;
On 2013/11/15 22:01:32, sky wrote:
> nit: url (to match url in .cc), and url is more descriptive too.

Done.

[sadrul, 2013-11-15 23:42:33]

Committed patchset #5 manually as r235469 (presubmit successful).