Don't update color in tabSwitcher Mode

Don't update color in tabSwitcher Mode

Fixes a few routing issues in theme color now that
we have multiple toolbars that may use color. Keeps
the dataprovider always up to date with the theme color, but
avoids visual updates in toolbar if it is not needed.

BUG=489545
Committed: https://crrev.com/5fc3c69424635dd98169f96653f6ef30e338406a
Cr-Commit-Position: refs/heads/master@{#331436}

[Yusuf, 2015-05-22 21:55:00]

yusufo@chromium.org changed reviewers:
+ tedchoc@chromium.org

[Ted C, 2015-05-26 18:44:35]

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
File
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
(left):

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java:643:
boolean colorChanged = mToolbarModel.getPrimaryColor() != color;
why did this need to be removed?

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java:647:
mToolbar.onPrimaryColorChanged();
I think the previous name was clearer and more consistent.  It is up to the
toolbar to act on signals from the manager, and it is fine for one to ignore an
update.  There are other places that do this and the naming for them is more
along the lines of updateBlah or onBlahChanged, so creating a new naming scheme
seems to add more mental confusion than necessary.

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
File
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarModelImpl.java
(right):

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarModelImpl.java:109:
int defaultColor =
context.getResources().getColor(R.color.default_primary_color);
This logic doesn't look like it changed.  just making it clearer?

[Yusuf, 2015-05-26 18:57:23]

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
File
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
(left):

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java:643:
boolean colorChanged = mToolbarModel.getPrimaryColor() != color;
On 2015/05/26 18:44:35, Ted C wrote:
> why did this need to be removed?

Changing this flow so that we now always get color pushed to the toolbar even if
the color doesn't change. That is why I wanted to rename it to reflect the
possibility of the color actually not changing.

Toolbar gets to decide whether it needs to early return or do something else.
This is to support the model being updated whenever the color changes, but this
getting reflected in the UI only when visual state changes. Say we are in
tabswitcher mode and the user selected a colored page. We update the model, but
this doesn't change the color of the toolbar. Then on refreshSelectedTab when we
switch out of tabswitcher, we check it again and transition if necessary.

Or we can find a way to handle all this in toolbar where it always does
something similar calling onPrimaryColorChanged for
updateVisualsForToolbarState.

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
File
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarModelImpl.java
(right):

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarModelImpl.java:109:
int defaultColor =
context.getResources().getColor(R.color.default_primary_color);
On 2015/05/26 18:44:35, Ted C wrote:
> This logic doesn't look like it changed.  just making it clearer?

Yes, just cleanup here.

[Yusuf, 2015-05-26 20:43:19]

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
File
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
(left):

https://codereview.chromium.org/1149213003/diff/1/chrome/android/java_staging...
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java:643:
boolean colorChanged = mToolbarModel.getPrimaryColor() != color;
On 2015/05/26 18:57:23, Yusuf wrote:
> On 2015/05/26 18:44:35, Ted C wrote:
> > why did this need to be removed?
> 
> Changing this flow so that we now always get color pushed to the toolbar even
if
> the color doesn't change. That is why I wanted to rename it to reflect the
> possibility of the color actually not changing.
> 
> Toolbar gets to decide whether it needs to early return or do something else.
> This is to support the model being updated whenever the color changes, but
this
> getting reflected in the UI only when visual state changes. Say we are in
> tabswitcher mode and the user selected a colored page. We update the model,
but
> this doesn't change the color of the toolbar. Then on refreshSelectedTab when
we
> switch out of tabswitcher, we check it again and transition if necessary.
> 
> Or we can find a way to handle all this in toolbar where it always does
> something similar calling onPrimaryColorChanged for
> updateVisualsForToolbarState.

Removed all the changes related with this since they were not necessary for the
fix. Just kept the bit in the toolbar phone update.

[Ted C, 2015-05-26 20:50:33]

lgtm

[Yusuf, 2015-05-26 20:52:43]

The CQ bit was checked by yusufo@chromium.org

[commit-bot: I haz the power, 2015-05-26 20:54:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1149213003/20001

[commit-bot: I haz the power, 2015-05-26 21:10:34]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-26 21:12:24]

Patchset 2 (id:??) landed as
https://crrev.com/5fc3c69424635dd98169f96653f6ef30e338406a
Cr-Commit-Position: refs/heads/master@{#331436}