Add RTL support to custom tabs toolbar

Add RTL support to custom tabs toolbar

This CL creates a special framelayout that groups security icon, urlbar
and titlebar together, and it ensures the security icon is always left
to the url bar.

BUG=506326
Committed: https://crrev.com/a422488f2ccb95791b88256247faffa5fd7731b0
Cr-Commit-Position: refs/heads/master@{#338108}

[Ian Wen, 2015-07-09 00:04:23]

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

[Yusuf, 2015-07-09 00:22:19]

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/res...
File chrome/android/java/res/layout/custom_tabs_toolbar.xml (right):

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/res...
chrome/android/java/res/layout/custom_tabs_toolbar.xml:14:
android:layout_width="match_parent"
the need for this confuses me. What happens when you set this to wrap_content?

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java
(right):

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java:69:
public static class UrlInfoLayout extends FrameLayout {
I think we can make do without the customized layout here.

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java:88:
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
why not have the same in onMeasure for toolbar layout? Adding this to
updateLayoutParams() would work to same effect and we already have the views
cached there.

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java:98:
private View mUrlInfoContainer;
let's use a new name for this since these became really confusing now. Lets call
the container for all three(security button, title and url)
mLocationBarFrameLayout.

[Ian Wen, 2015-07-09 01:19:32]

Thanks for the comments. All done.

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/res...
File chrome/android/java/res/layout/custom_tabs_toolbar.xml (right):

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/res...
chrome/android/java/res/layout/custom_tabs_toolbar.xml:14:
android:layout_width="match_parent"
On 2015/07/09 00:22:19, Yusuf wrote:
> the need for this confuses me. What happens when you set this to wrap_content?

With or without it does not seem to affect anything. Reverted back to
wrap_contents.

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java
(right):

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java:69:
public static class UrlInfoLayout extends FrameLayout {
On 2015/07/09 00:22:19, Yusuf wrote:
> I think we can make do without the customized layout here.

Oh yeah. Removed.

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java:88:
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
On 2015/07/09 00:22:19, Yusuf wrote:
> why not have the same in onMeasure for toolbar layout? Adding this to
> updateLayoutParams() would work to same effect and we already have the views
> cached there.

Done.

https://codereview.chromium.org/1226183002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java:98:
private View mUrlInfoContainer;
On 2015/07/09 00:22:19, Yusuf wrote:
> let's use a new name for this since these became really confusing now. Lets
call
> the container for all three(security button, title and url)
> mLocationBarFrameLayout.

Done.

[Yusuf, 2015-07-09 17:30:42]

https://codereview.chromium.org/1226183002/diff/40001/chrome/android/java/res...
File chrome/android/java/res/layout/custom_tabs_toolbar.xml (right):

https://codereview.chromium.org/1226183002/diff/40001/chrome/android/java/res...
chrome/android/java/res/layout/custom_tabs_toolbar.xml:29:
android:layout_width="match_parent"
how about this one? Do we need match_parent here?

https://codereview.chromium.org/1226183002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java
(right):

https://codereview.chromium.org/1226183002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java:95:
mLocationBarFrameLayout = findViewById(R.id.url_info_container);
rename the id as well

[Ian Wen, 2015-07-09 18:09:48]

https://codereview.chromium.org/1226183002/diff/40001/chrome/android/java/res...
File chrome/android/java/res/layout/custom_tabs_toolbar.xml (right):

https://codereview.chromium.org/1226183002/diff/40001/chrome/android/java/res...
chrome/android/java/res/layout/custom_tabs_toolbar.xml:29:
android:layout_width="match_parent"
On 2015/07/09 17:30:42, Yusuf wrote:
> how about this one? Do we need match_parent here?

Talked offline. match_parent here and above are required to give enough space
for translation animation.

https://codereview.chromium.org/1226183002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java
(right):

https://codereview.chromium.org/1226183002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java:95:
mLocationBarFrameLayout = findViewById(R.id.url_info_container);
On 2015/07/09 17:30:42, Yusuf wrote:
> rename the id as well

Done. Renamed other old names as well.

[Yusuf, 2015-07-09 18:14:20]

lgtm

[Ian Wen, 2015-07-09 18:15:21]

The CQ bit was checked by ianwen@chromium.org

[commit-bot: I haz the power, 2015-07-09 18:17:04]

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

[commit-bot: I haz the power, 2015-07-09 19:01:28]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-07-09 19:02:23]

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