Fix showing some foreground tabs.

Fix showing some foreground tabs.

Some tabs (e.g. 'view source', opening a bookmark from the bookmark bar's
context menu) open in background, although they are supposed to open as
foreground tabs. This small changeset fixes the problem.

BUG=61735
TEST=see bug

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

[sky, 2010-11-08 17:19:49]

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

http://codereview.chromium.org/4534002/diff/1/2#newcode2712
chrome/browser/browser.cc:2712: params.tabstrip_add_types = disposition ==
NEW_FOREGROUND_TAB ?
I tend to think NEW_FOREGROUND_TAB should force ADD_SELECTED in
NormalizeDisposition. Ben, what do you think?

[Ben Goodger (Google), 2010-11-08 17:45:56]

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

http://codereview.chromium.org/4534002/diff/1/2#newcode2712
chrome/browser/browser.cc:2712: params.tabstrip_add_types = disposition ==
NEW_FOREGROUND_TAB ?
On 2010/11/08 17:19:49, sky wrote:
> I tend to think NEW_FOREGROUND_TAB should force ADD_SELECTED in
> NormalizeDisposition. Ben, what do you think?

Agreed.

[Ben Goodger (Google), 2010-11-08 17:50:48]

yes.

On Mon, Nov 8, 2010 at 9:48 AM, <sadrul@chromium.org> wrote:

> > I tend to think NEW_FOREGROUND_TAB should force ADD_SELECTED in
>> > NormalizeDisposition. Ben, what do you think?
>>
>
>  Agreed.
>>
>
> There are a number of places where _add_types is set to ADD_SELECTED for
> NEW_FOREGROUND_TAB in browser.cc. Should I replace all those instances too?
>
>
> http://codereview.chromium.org/4534002/show
>

[sadrul, 2010-11-08 19:40:23]

Done.

[sadrul, 2010-11-09 15:09:02]

Merged with trunk.

[Ben Goodger (Google), 2010-11-09 15:49:38]

Didn't you say you had to patch a few call sites?

[sadrul, 2010-11-09 15:55:27]

On 2010/11/09 15:49:38, Ben Goodger wrote:
> Didn't you say you had to patch a few call sites?

The removal from browser.cc in this CL seems to be the only one that can be
removed. There are a couple of other places where ADD_SELECTED is explicitly
used (e.g. lines 977, 1111, 4078), but those do not seem to be replaceable
(since the callbacks don't see the disposition).

[Ben Goodger (Google), 2010-11-09 16:02:12]

Very well. LGTM if the tests pass.

[sadrul, 2010-11-09 16:15:32]

The trybots don't seem to have the revision that moved browser.cc into ui/ yet.
That's why they are failing. I will resend when the trybots get newer revs.