[OriginChip] Re-enable the chip as necessary when switching tabs.

[OriginChip] Re-enable the chip as necessary when switching tabs.

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

[Justin Donnelly, 2014-05-16 16:56:16]

It's increasingly looking to me like url_replacement_enabled and
origin_chip_enabled should always be in the same state (i.e. we don't need a
separate origin_chip_enabled toggle).

[Peter Kasting, 2014-05-16 17:57:44]

Regarding your comment, I would try and define, in the code, what "url
replacement enabled" means, and what "origin chip enabled" means (for this
particular toggle).  For example, when we have no origin chip but search term
replacement is on, are there cases where we try not to turn on "url replacement
enabled", because the page isn't an SRP?  Those would represent cases where
these two toggles would differ.

It may be that we intended these toggles to be on much more often than we
thought we'd show the origin chip or do search term replacement, and they're
just trying to control things like "don't do this while the user is editing". 
If that's the case, I think your instinct is correct that we can combine them,
unless it would make more sense to combine each with the more precise functions
that say "should you actually be doing X right now".

Anyway, this change seems likely to be more correct for now, even if I'm not
sure I totally understand how this works anymore, so LGTM.  It would be nice to
have tests for this, though.

[Justin Donnelly, 2014-05-16 18:58:01]

On 2014/05/16 17:57:44, Peter Kasting wrote:
> Regarding your comment, I would try and define, in the code, what "url
> replacement enabled" means, and what "origin chip enabled" means (for this
> particular toggle).  For example, when we have no origin chip but search term
> replacement is on, are there cases where we try not to turn on "url
replacement
> enabled", because the page isn't an SRP?  Those would represent cases where
> these two toggles would differ.
> 
> It may be that we intended these toggles to be on much more often than we
> thought we'd show the origin chip or do search term replacement, and they're
> just trying to control things like "don't do this while the user is editing". 
> If that's the case, I think your instinct is correct that we can combine them,
> unless it would make more sense to combine each with the more precise
functions
> that say "should you actually be doing X right now".
> 
> Anyway, this change seems likely to be more correct for now, even if I'm not
> sure I totally understand how this works anymore, so LGTM.  It would be nice
to
> have tests for this, though.

Thanks for the feedback.  I'll investigate further at some point.  Submitting
this change for now.

[Justin Donnelly, 2014-05-16 18:58:09]

The CQ bit was checked by jdonnelly@chromium.org

[commit-bot: I haz the power, 2014-05-16 18:59:02]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdonnelly@chromium.org/292493003/1

[commit-bot: I haz the power, 2014-05-17 04:54:09]

Change committed as 271161