Upstream SpanApplier.java and SpanApplierTest.java

Upstream SpanApplier.java and SpanApplierTest.java

Modify build files to enable instrumentation tests for org.chromium.ui

BUG=432158
Committed: https://crrev.com/087c46f79264a9760ef3a029255547e084003e9e
Cr-Commit-Position: refs/heads/master@{#305094}

[Theresa, 2014-11-19 23:25:50]

twellington@chromium.org changed reviewers:
+ newt@chromium.org, tedchoc@chromium.org

[newt (away), 2014-11-19 23:35:37]

Java files look good to me. Ted or cjhopman@ should look at the build changes
though.

[Ted C, 2014-11-20 18:38:53]

lgtm w/ some nits

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
File ui/android/java/src/org/chromium/ui/text/SpanApplier.java (right):

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/text/SpanApplier.java:29: final String
mStartTag;
why are these protected instead of private?

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/text/SpanApplier.java:41: this.mStartTag =
startTag;
this. isn't necessary since the variable names are different

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/text/SpanApplier.java:55: return false;
put this on the previous line (braces always required in java unless the
statement and condition can all fit on a single line)

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/text/SpanApplier.java:62: return 0;
You can just use eclipse's hashCode generator to make this more useful:

        @Override
        public int hashCode() {
            final int prime = 31;
            int result = 1;
            result = prime * result + ((mEndTag == null) ? 0 :
mEndTag.hashCode());
            result = prime * result + mEndTagIndex;
            result = prime * result + ((mSpan == null) ? 0 : mSpan.hashCode());
            result = prime * result + ((mStartTag == null) ? 0 :
mStartTag.hashCode());
            result = prime * result + mStartTagIndex;
            return result;
        }

[newt (away), 2014-11-20 18:49:22]

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
File ui/android/java/src/org/chromium/ui/text/SpanApplier.java (right):

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/text/SpanApplier.java:29: final String
mStartTag;
On 2014/11/20 18:38:53, Ted C wrote:
> why are these protected instead of private?

Because they're accessed by the outer class. They could be made private, but
then Java has to create a bunch of hidden methods to allow the outer class to
access these fields. My preference was for the former option.

In any case, we should be consistent and make the member fields all package
protected or all private.

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/text/SpanApplier.java:62: return 0;
On 2014/11/20 18:38:53, Ted C wrote:
> You can just use eclipse's hashCode generator to make this more useful:
> 
>         @Override
>         public int hashCode() {
>             final int prime = 31;
>             int result = 1;
>             result = prime * result + ((mEndTag == null) ? 0 :
> mEndTag.hashCode());
>             result = prime * result + mEndTagIndex;
>             result = prime * result + ((mSpan == null) ? 0 :
mSpan.hashCode());
>             result = prime * result + ((mStartTag == null) ? 0 :
> mStartTag.hashCode());
>             result = prime * result + mStartTagIndex;
>             return result;
>         }

This method isn't used. It was implemented so that findbugs would stop
complaining that "if you implement equals(), you should generally implement
hashCode()". BTW, "return 0" is a valid, if inefficient, implementation of
hashCode().

[Theresa, 2014-11-20 19:51:38]

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
File ui/android/java/src/org/chromium/ui/text/SpanApplier.java (right):

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/text/SpanApplier.java:29: final String
mStartTag;
On 2014/11/20 18:49:22, newt wrote:
> On 2014/11/20 18:38:53, Ted C wrote:
> > why are these protected instead of private?
> 
> Because they're accessed by the outer class. They could be made private, but
> then Java has to create a bunch of hidden methods to allow the outer class to
> access these fields. My preference was for the former option.
> 
> In any case, we should be consistent and make the member fields all package
> protected or all private.


I made them all package protected for consistency.

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/text/SpanApplier.java:41: this.mStartTag =
startTag;
On 2014/11/20 18:38:53, Ted C wrote:
> this. isn't necessary since the variable names are different

Done.

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/text/SpanApplier.java:55: return false;
On 2014/11/20 18:38:53, Ted C wrote:
> put this on the previous line (braces always required in java unless the
> statement and condition can all fit on a single line)

Done.

https://codereview.chromium.org/736163002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/text/SpanApplier.java:62: return 0;
On 2014/11/20 18:49:22, newt wrote:
> On 2014/11/20 18:38:53, Ted C wrote:
> > You can just use eclipse's hashCode generator to make this more useful:
> > 
> >         @Override
> >         public int hashCode() {
> >             final int prime = 31;
> >             int result = 1;
> >             result = prime * result + ((mEndTag == null) ? 0 :
> > mEndTag.hashCode());
> >             result = prime * result + mEndTagIndex;
> >             result = prime * result + ((mSpan == null) ? 0 :
> mSpan.hashCode());
> >             result = prime * result + ((mStartTag == null) ? 0 :
> > mStartTag.hashCode());
> >             result = prime * result + mStartTagIndex;
> >             return result;
> >         }
> 
> This method isn't used. It was implemented so that findbugs would stop
> complaining that "if you implement equals(), you should generally implement
> hashCode()". BTW, "return 0" is a valid, if inefficient, implementation of
> hashCode().

Acknowledged.

[Theresa, 2014-11-20 21:47:39]

The CQ bit was checked by twellington@chromium.org

[commit-bot: I haz the power, 2014-11-20 21:48:41]

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

[commit-bot: I haz the power, 2014-11-20 22:44:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-11-20 22:45:51]

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