diff options
author | Yigit Boyar <yboyar@google.com> | 2015-08-25 17:08:37 -0700 |
---|---|---|
committer | Yigit Boyar <yboyar@google.com> | 2015-08-26 18:00:21 -0700 |
commit | eebcbdd5d35e56a2c8ef37feeb65df46130d001d (patch) | |
tree | 8c191f5d9bdb5840d6d8291d1da82004493197ab /integration-tests | |
parent | bd42d20f70b1f88e27e3b3c9c3a9c55ec155d128 (diff) | |
download | data-binding-eebcbdd5d35e56a2c8ef37feeb65df46130d001d.tar.gz |
Fix the bug about marking expressions as read early
@{obj4.text}
@{obj4.useHello ? obj4.text : `hello`}
This case was broken and would not re-read obj4.text if
only obj4.useHello is invalidated. It was partially fixed in
Change-Id: Id449c8819b8dc0301a7ab893002478914780d480 but
but it was bringing it down to exact equality which would
mean we could fail to mark sth as read.
The coverage logic we use in expressions when marking them
as read was giving false positives, which results in
marking expressions as read before they are fully read.
This CL fixes that bug. The safe fix introduces some false
negatives when a conditional is behind another conditional.
We can address this post v1.
There was also another bug about setting conditional flags
even though the ternary does not need to be calculated.
@{obja.boolMethod(a)}
@{a ? objb.boolMethod(b) : objc.boolMethod(c)}
When obja is invalidated, it would re calculate the second
binding expression too even though it is never used (because
that expression is not invalidated). The re-calculation would
happen because we would calculate the value of `a` and set
the conditional flags w/o checking invalidation.
This would result in unnecessary calculations. I've also fixed
it for first degree where the ternary is not under another
ternary. The proper fix would requires bigger effort, post V1.
bug: 22957203
Change-Id: Ib73f31eac358f2ad7652395a021baaa93b79adf7
Diffstat (limited to 'integration-tests')
3 files changed, 117 insertions, 0 deletions
diff --git a/integration-tests/TestApp/app/src/androidTestApi7/java/android/databinding/testapp/UnnecessaryCalculationTest.java b/integration-tests/TestApp/app/src/androidTestApi7/java/android/databinding/testapp/UnnecessaryCalculationTest.java new file mode 100644 index 00000000..986156c1 --- /dev/null +++ b/integration-tests/TestApp/app/src/androidTestApi7/java/android/databinding/testapp/UnnecessaryCalculationTest.java @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package android.databinding.testapp; + +import android.databinding.testapp.databinding.UnnecessaryCalculationBinding; +import android.databinding.testapp.vo.BasicObject; +import android.support.annotation.UiThread; +import android.test.UiThreadTest; + +import java.util.concurrent.atomic.AtomicInteger; + +public class UnnecessaryCalculationTest extends BaseDataBinderTest<UnnecessaryCalculationBinding> { + + public UnnecessaryCalculationTest() { + super(UnnecessaryCalculationBinding.class); + } + + @Override + protected void setUp() throws Exception { + super.setUp(); + initBinder(); + } + + @UiThreadTest + public void testDontSetUnnecessaryFlags() { + BasicObjWithCounter obja = new BasicObjWithCounter(); + BasicObjWithCounter objb = new BasicObjWithCounter(); + BasicObjWithCounter objc = new BasicObjWithCounter(); + mBinder.setObja(obja); + mBinder.setObjb(objb); + mBinder.setObjc(objc); + mBinder.setA(true); + mBinder.setB(true); + mBinder.setC(false); + mBinder.executePendingBindings(); + assertEquals("true", mBinder.textView.getText().toString()); + assertEquals("true", mBinder.textView2.getText().toString()); + assertEquals(1, obja.counter); + assertEquals(1, objb.counter); + assertEquals(0, objc.counter); + obja = new BasicObjWithCounter(); + mBinder.setObja(obja); + mBinder.executePendingBindings(); + assertEquals("true", mBinder.textView.getText().toString()); + assertEquals("true", mBinder.textView2.getText().toString()); + assertEquals(1, obja.counter); + assertEquals(1, objb.counter); + assertEquals(0, objc.counter); + } + + private static class BasicObjWithCounter extends BasicObject { + int counter = 0; + + @Override + public String boolMethod(boolean value) { + counter ++; + return super.boolMethod(value); + } + } + +} diff --git a/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java b/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java index 450f7fb1..4ec76c5a 100644 --- a/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java +++ b/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java @@ -42,4 +42,8 @@ public class BasicObject extends BaseObservable { this.mField2 = field2; notifyPropertyChanged(BR.field1); } + + public String boolMethod(boolean value) { + return value ? "true" : "false"; + } } diff --git a/integration-tests/TestApp/app/src/main/res/layout/unnecessary_calculation.xml b/integration-tests/TestApp/app/src/main/res/layout/unnecessary_calculation.xml new file mode 100644 index 00000000..0410189b --- /dev/null +++ b/integration-tests/TestApp/app/src/main/res/layout/unnecessary_calculation.xml @@ -0,0 +1,39 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- + ~ Copyright (C) 2015 The Android Open Source Project + ~ Licensed under the Apache License, Version 2.0 (the "License"); + ~ you may not use this file except in compliance with the License. + ~ You may obtain a copy of the License at + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> + +<layout xmlns:android="http://schemas.android.com/apk/res/android"> + <data> + <variable name="obja" type="android.databinding.testapp.vo.BasicObject"/> + <variable name="objb" type="android.databinding.testapp.vo.BasicObject"/> + <variable name="objc" type="android.databinding.testapp.vo.BasicObject"/> + <variable name="a" type="boolean"/> + <variable name="b" type="boolean"/> + <variable name="c" type="boolean"/> + </data> + <LinearLayout + android:orientation="vertical" + android:layout_width="match_parent" + android:layout_height="match_parent"> + <TextView + android:id="@+id/textView" + android:text="@{obja.boolMethod(a)}" + android:layout_width="wrap_content" + android:layout_height="wrap_content"/> + <android.databinding.testapp.view.MyTextView + android:id="@+id/textView2" + android:text="@{a ? objb.boolMethod(b) : objc.boolMethod(c)}" + android:layout_width="wrap_content" + android:layout_height="wrap_content"/> + </LinearLayout> +</layout>
\ No newline at end of file |