Remove attempt to be smart about where to open navigations

Remove attempt to be smart about where to open navigations
targetting app tabs. I futzed with this a bit to try and
integrate installed apps extents, but it is complex.

Also, I looked over the original bug where this code was
added: crbug.com/29281. The issue there was that when you
click on a bookmark (presumably in the bookmark bar) the
pinned tab is unexpectedly overwritten. I can see that
complaint, but there is also the use case of clicking
a bookmark and intending it to overwrite the pinned tab.

In summary, I kinda feel like the expectations about
how navigation should work are too ingrained and we
shouldn't be meddling.

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

[Peter Kasting, 2010-08-20 23:29:37]

The original changes also added
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/ext...
.  I don't know if that can/should be reverted.

http://codereview.chromium.org/3161037/diff/1/2
File chrome/browser/browser.cc (right):

http://codereview.chromium.org/3161037/diff/1/2#newcode4042
chrome/browser/browser.cc:4042: bool Browser::IsPinned(TabContents* source) {
This function can die too, I think.

http://codereview.chromium.org/3161037/diff/1/3
File chrome/browser/browser.h (right):

http://codereview.chromium.org/3161037/diff/1/3#newcode639
chrome/browser/browser.h:639: bool IsPinned(TabContents* source);
This function can die too, I think.

[Aaron Boodman, 2010-08-21 00:23:55]

On 2010/08/20 23:29:37, Peter Kasting wrote:
> The original changes also added
>
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/ext...
> .  I don't know if that can/should be reverted.

Thank you for bringing that to my attention. That code was incorrect. I've added
a quick fix to it in this change.

> http://codereview.chromium.org/3161037/diff/1/2
> File chrome/browser/browser.cc (right):
> 
> http://codereview.chromium.org/3161037/diff/1/2#newcode4042
> chrome/browser/browser.cc:4042: bool Browser::IsPinned(TabContents* source) {
> This function can die too, I think.
> 
> http://codereview.chromium.org/3161037/diff/1/3
> File chrome/browser/browser.h (right):
> 
> http://codereview.chromium.org/3161037/diff/1/3#newcode639
> chrome/browser/browser.h:639: bool IsPinned(TabContents* source);
> This function can die too, I think.

Done.

[Peter Kasting, 2010-08-21 02:48:21]

LGTM

[Evan Stade, 2010-09-07 18:56:14]

I strongly feel as if I and the original reviewers should have been looped in on
this.

Better yet, get a decision from a UI lead.

[Peter Kasting, 2010-09-07 19:07:35]

On 2010/09/07 18:56:14, Evan Stade wrote:
> I strongly feel as if I and the original reviewers should have been looped in
on
> this.

Sorry, I thought much of this had been implemented by sky?  I know he and I
talked about the original design and implementation for hours, and he was on the
bug attached to this patch.

It wasn't my intent to leave out stakeholders.

[Evan Stade, 2010-09-07 19:27:03]

I am partially to blame for letting this slip beneath my radar (guess it's a
result of bugtracker CC spam).

In general I feel that where there is disagreement, we ought to go through the
formal process of getting UI leads to decide. Although, actually testing this,
it leaves the bookmark bar navigations alone (i.e. they still open in a new tab
when clicked in a pinned tab), so I'm confused by the description on this CL.

[Evan Stade, 2010-09-07 19:38:03]

On 2010/09/07 19:27:03, Evan Stade wrote:
> (i.e. they still open in a new tab when clicked in a pinned tab)

nevermind. It does what it says. So anyway I don't believe it's a good idea to
fix one behavior by breaking another (this applies to the original patch and its
revert).