Upstream CompositorButton, VirtualView and TitleBitmapFactory

Upstream CompositorButton, VirtualView and TitleBitmapFactory

BUG=428963
Committed: https://crrev.com/9cf105cba8e35d51273d52d4e3a5b835d701e21a
Cr-Commit-Position: refs/heads/master@{#311123}

[Jaekyun Seok (inactive), 2015-01-09 03:00:39]

jaekyun@chromium.org changed reviewers:
+ changwan@chromium.org, dtrainor@chromium.org

[Jaekyun Seok (inactive), 2015-01-09 03:00:39]

Please review this CL.

[David Trainor- moved to gerrit, 2015-01-09 18:18:41]

On 2015/01/09 03:00:39, Jaekyun Seok wrote:
> Please review this CL.

Can we move those java files somewhere that makes more sense?  I know that's
where they are now but the package never really matched what the files were
(none have to do with tabs in particular, and they're all used by the
compositor).

I would suggest something like the following:
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/components/CompositorButton.java
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/content/TitleBitmapFactory.java
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/components/VirtualView.java

[David Trainor- moved to gerrit, 2015-01-09 18:18:51]

Also some nits

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File chrome/android/java/res/values-sw600dp/values.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values-sw600dp/values.xml:33: <!-- TabStrip text -->
Compositor Tab Title Text

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File chrome/android/java/res/values-xhdpi/dimens.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values-xhdpi/dimens.xml:10: <!-- The size of the
boudning box for favicons in the tab switcher. -->
Remove "in the tab switcher"

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File chrome/android/java/res/values/colors.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/colors.xml:72: <!-- Tab Switcher Colors -->
Compositor Tab Title Colors

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File chrome/android/java/res/values/dimens.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/dimens.xml:113: <!-- TabStrip resources -->
Compositor Tab Title Dimensions?

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/dimens.xml:114: <dimen
name="tab_title_bar_shadow_x_offset">0.6667dp</dimen>            <!-- 1px hdpi
-->
compositor_tab_title_*

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/dimens.xml:118: <!-- The size of the text for tab
titles in the switcher view. -->
Fix this to remove switcher view.  Same with all other comments in these files.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/dimens.xml:123: <dimen
name="close_button_slop">10dp</dimen>
compositor_button_slop?  It's used by the default CompositorButton not only
close buttons.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File chrome/android/java/res/values/values.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/values.xml:33: <!-- TabStrip text -->
Compositor Tab Title Text

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File
chrome/android/java/src/org/chromium/chrome/browser/tabs/TitleBitmapFactory.java
(right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/src/org/chromium/chrome/browser/tabs/TitleBitmapFactory.java:25:
* Bitmap factory for tab title bitmap. This is only relevant for the phone tab
switcher.
Fix the comment: A factory that creates text and favicon bitmaps.

[Jaekyun Seok (inactive), 2015-01-09 20:37:22]

PTAL.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File chrome/android/java/res/values-sw600dp/values.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values-sw600dp/values.xml:33: <!-- TabStrip text -->
On 2015/01/09 18:18:51, David Trainor wrote:
> Compositor Tab Title Text

Done.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File chrome/android/java/res/values-xhdpi/dimens.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values-xhdpi/dimens.xml:10: <!-- The size of the
boudning box for favicons in the tab switcher. -->
On 2015/01/09 18:18:51, David Trainor wrote:
> Remove "in the tab switcher"

Done.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File chrome/android/java/res/values/colors.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/colors.xml:72: <!-- Tab Switcher Colors -->
On 2015/01/09 18:18:51, David Trainor wrote:
> Compositor Tab Title Colors

Done.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File chrome/android/java/res/values/dimens.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/dimens.xml:113: <!-- TabStrip resources -->
On 2015/01/09 18:18:51, David Trainor wrote:
> Compositor Tab Title Dimensions?

Done.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/dimens.xml:114: <dimen
name="tab_title_bar_shadow_x_offset">0.6667dp</dimen>            <!-- 1px hdpi
-->
On 2015/01/09 18:18:51, David Trainor wrote:
> compositor_tab_title_*

Done.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/dimens.xml:118: <!-- The size of the text for tab
titles in the switcher view. -->
On 2015/01/09 18:18:51, David Trainor wrote:
> Fix this to remove switcher view.  Same with all other comments in these
files.

Done.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/dimens.xml:123: <dimen
name="close_button_slop">10dp</dimen>
On 2015/01/09 18:18:51, David Trainor wrote:
> compositor_button_slop?  It's used by the default CompositorButton not only
> close buttons.

Done.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File chrome/android/java/res/values/values.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/res/values/values.xml:33: <!-- TabStrip text -->
On 2015/01/09 18:18:51, David Trainor wrote:
> Compositor Tab Title Text

Done.

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
File
chrome/android/java/src/org/chromium/chrome/browser/tabs/TitleBitmapFactory.java
(right):

https://chromiumcodereview.appspot.com/842083003/diff/20001/chrome/android/ja...
chrome/android/java/src/org/chromium/chrome/browser/tabs/TitleBitmapFactory.java:25:
* Bitmap factory for tab title bitmap. This is only relevant for the phone tab
switcher.
On 2015/01/09 18:18:51, David Trainor wrote:
> Fix the comment: A factory that creates text and favicon bitmaps.

Done.

[Jaekyun Seok (inactive), 2015-01-12 00:46:50]

Ping?

[Changwan Ryu, 2015-01-12 09:35:26]

https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
File chrome/android/java/res/values/dimens.xml (right):

https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
chrome/android/java/res/values/dimens.xml:117: <dimen
name="compositor_tab_title_bar_shadow_x_offset">0.6667dp</dimen>            <!--
1px hdpi -->
line limit? (all the following lines, too)

https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
chrome/android/java/res/values/dimens.xml:126: <dimen
name="compositor_button_slop">10dp</dimen>
This seems not specific to tab switcher. Could you remove TabSwitcher from the
comment?

[Changwan Ryu, 2015-01-12 09:44:34]

On 2015/01/12 09:35:26, Changwan Ryu wrote:
>
https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
> File chrome/android/java/res/values/dimens.xml (right):
> 
>
https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
> chrome/android/java/res/values/dimens.xml:117: <dimen
> name="compositor_tab_title_bar_shadow_x_offset">0.6667dp</dimen>           
<!--
> 1px hdpi -->
> line limit? (all the following lines, too)
> 
>
https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
> chrome/android/java/res/values/dimens.xml:126: <dimen
> name="compositor_button_slop">10dp</dimen>
> This seems not specific to tab switcher. Could you remove TabSwitcher from the
> comment?

Also, the CL title / description should be updated.

[Jaekyun Seok (inactive), 2015-01-12 10:04:01]

https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
File chrome/android/java/res/values/dimens.xml (right):

https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
chrome/android/java/res/values/dimens.xml:117: <dimen
name="compositor_tab_title_bar_shadow_x_offset">0.6667dp</dimen>            <!--
1px hdpi -->
On 2015/01/12 09:35:26, Changwan Ryu wrote:
> line limit? (all the following lines, too)

Is there any specific length limit?

https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
chrome/android/java/res/values/dimens.xml:126: <dimen
name="compositor_button_slop">10dp</dimen>
On 2015/01/12 09:35:26, Changwan Ryu wrote:
> This seems not specific to tab switcher. Could you remove TabSwitcher from the
> comment?

I will.

[Changwan Ryu, 2015-01-12 10:15:46]

https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
File chrome/android/java/res/values/dimens.xml (right):

https://codereview.chromium.org/842083003/diff/40001/chrome/android/java/res/...
chrome/android/java/res/values/dimens.xml:117: <dimen
name="compositor_tab_title_bar_shadow_x_offset">0.6667dp</dimen>            <!--
1px hdpi -->
On 2015/01/12 10:04:01, Jaekyun Seok wrote:
> On 2015/01/12 09:35:26, Changwan Ryu wrote:
> > line limit? (all the following lines, too)
> 
> Is there any specific length limit?

Hmm... Never mind. The copyright statement keeps it at 80, but I don't think
there is a consistent line limit applied to xml files.

[Jaekyun Seok (inactive), 2015-01-12 10:18:16]

> Hmm... Never mind. The copyright statement keeps it at 80, but I don't think
> there is a consistent line limit applied to xml files.

I see. I've uploaded a new patch after removing the "TabSwitcher", and modified
the title of this CL.

PTAL.

[David Trainor- moved to gerrit, 2015-01-12 19:26:08]

lgtm

https://chromiumcodereview.appspot.com/842083003/diff/60001/chrome/android/ja...
File chrome/android/java/res/values/dimens.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/60001/chrome/android/ja...
chrome/android/java/res/values/dimens.xml:123: <!-- The size of the boudning box
for favicons. -->
boudning -> bounding

[Jaekyun Seok (inactive), 2015-01-12 21:22:41]

https://chromiumcodereview.appspot.com/842083003/diff/60001/chrome/android/ja...
File chrome/android/java/res/values/dimens.xml (right):

https://chromiumcodereview.appspot.com/842083003/diff/60001/chrome/android/ja...
chrome/android/java/res/values/dimens.xml:123: <!-- The size of the boudning box
for favicons. -->
On 2015/01/12 19:26:08, David Trainor wrote:
> boudning -> bounding

Done.

[Jaekyun Seok (inactive), 2015-01-12 21:22:49]

The CQ bit was checked by jaekyun@chromium.org

[commit-bot: I haz the power, 2015-01-12 21:23:53]

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

[commit-bot: I haz the power, 2015-01-12 22:20:21]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-01-12 22:21:22]

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

[Nico, 2015-07-22 20:15:50]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2015-07-22 20:15:51]

https://codereview.chromium.org/842083003/diff/80001/chrome/android/java/src/...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/content/TitleBitmapFactory.java
(right):

https://codereview.chromium.org/842083003/diff/80001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/content/TitleBitmapFactory.java:25:
* A factory that creates text and favicon bitmaps. This is only relevant for the
phone tab
I don't think this comment is true: This seems to be used for the tablet tab
strip too.

[Jaekyun Seok (inactive), 2015-07-22 21:35:55]

https://codereview.chromium.org/842083003/diff/80001/chrome/android/java/src/...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/content/TitleBitmapFactory.java
(right):

https://codereview.chromium.org/842083003/diff/80001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/content/TitleBitmapFactory.java:25:
* A factory that creates text and favicon bitmaps. This is only relevant for the
phone tab
On 2015/07/22 20:15:51, Nico (hiding) wrote:
> I don't think this comment is true: This seems to be used for the tablet tab
> strip too.

I will update this soon.