[Tracking Requested - why for this release]: Android build bustage when Gecko 36 merges to Beta. https://treeherder.mozilla.org/ui/logviewer.html#?job_id=2977389&repo=try gecko-util.jar /usr/bin/javac -target 1.7 -source 1.7 -classpath /builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/extras/google/google_play_services/libproject/google-play-services_lib/libs/google-play-services.jar:/builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/extras/android/support/v7/mediarouter/libs/android-support-v7-mediarouter.jar:/builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/extras/android/support/v7/appcompat/libs/android-support-v7-appcompat.jar -bootclasspath /builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/platforms/android-20/android.jar:/builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/extras/android/support/v4/android-support-v4.jar -encoding UTF8 -g:source,lines -Werror -Xlint:all,-deprecation -d gecko-util-classes -classpath gecko-mozglue.jar /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/ActivityResultHandler.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/ActivityResultHandlerMap.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/ActivityUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/Clipboard.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/EventCallback.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/FileUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/FloatUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/GamepadUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/GeckoBackgroundThread.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/GeckoEventListener.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/GeckoJarReader.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/GeckoRequest.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/HardwareUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/INIParser.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/INISection.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/IOUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/JSONUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/MenuUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/NativeEventListener.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/NativeJSContainer.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/NativeJSObject.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/NonEvictingLruCache.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/PrefUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/ProxySelector.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/RawResource.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/StringUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/ThreadUtils.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/UIAsyncTask.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/util/WebActivityMapper.java /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/resources/layout-v11/new_tablet_tabs_item_cell.xml:25: error: Error: No resource found that matches the given name (at 'textColor' with value '@color/new_tablet_tab_item_title'). /builds/slave/try-and-0000000000000000000000/build/mobile/android/base/resources/layout-v11/new_tablet_tabs_item_cell.xml:40: error: Error: No resource found that matches the given name (at 'src' with value '@drawable/new_tablet_tab_item_close_button'). aapt: warning: string 'auth_client_needs_enabling_title' has no default translation in /builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/extras/google/google_play_services/libproject/google-play-services_lib/res; found: af am ar be bg ca cs da de el en_GB en_IN es es_US et_EE fa fi fr fr_CA hi hr hu hy_AM in it iw ja ka_GE km_KH ko lo_LA lt lv mn_MN ms_MY nb nl pl pt pt_BR pt_PT ro ru sk sl sr sv sw th tl tr uk vi zh_CN zh_HK zh_TW zu aapt: warning: string 'auth_client_needs_installation_title' has no default translation in /builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/extras/google/google_play_services/libproject/google-play-services_lib/res; found: af am ar be bg ca cs da de el en_GB en_IN es es_US et_EE fa fi fr fr_CA hi hr hu hy_AM in it iw ja ka_GE km_KH ko lo_LA lt lv mn_MN ms_MY nb nl pl pt pt_BR pt_PT ro ru sk sl sr sv sw th tl tr uk vi zh_CN zh_HK zh_TW zu aapt: warning: string 'auth_client_needs_update_title' has no default translation in /builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/extras/google/google_play_services/libproject/google-play-services_lib/res; found: af am ar be bg ca cs da de el en_GB en_IN es es_US et_EE fa fi fr fr_CA hi hr hu hy_AM in it iw ja ka_GE km_KH ko lo_LA lt lv mn_MN ms_MY nb nl pl pt pt_BR pt_PT ro ru sk sl sr sv sw th tl tr uk vi zh_CN zh_HK zh_TW zu aapt: warning: string 'auth_client_play_services_err_notification_msg' has no default translation in /builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/extras/google/google_play_services/libproject/google-play-services_lib/res; found: af am ar be bg ca cs da de el en_GB en_IN es es_US et_EE fa fi fr fr_CA hi hr hu hy_AM in it iw ja ka_GE km_KH ko lo_LA lt lv mn_MN ms_MY nb nl pl pt pt_BR pt_PT ro ru sk sl sr sv sw th tl tr uk vi zh_CN zh_HK zh_TW zu aapt: warning: string 'auth_client_requested_by_msg' has no default translation in /builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/extras/google/google_play_services/libproject/google-play-services_lib/res; found: af am ar be bg ca cs da de el en_GB en_IN es es_US et_EE fa fi fr fr_CA hi hr hu hy_AM in it iw ja ka_GE km_KH ko lo_LA lt lv mn_MN ms_MY nb nl pl pt pt_BR pt_PT ro ru sk sl sr sv sw th tl tr uk vi zh_CN zh_HK zh_TW zu aapt: warning: string 'auth_client_using_bad_version_title' has no default translation in /builds/slave/try-and-0000000000000000000000/build/android-sdk-linux/extras/google/google_play_services/libproject/google-play-services_lib/res; found: af am ar be bg ca cs da de el en_GB en_IN es es_US et_EE fa fi fr fr_CA hi hr hu hy_AM in it iw ja ka_GE km_KH ko lo_LA lt lv mn_MN ms_MY nb nl pl pt pt_BR pt_PT ro ru sk sl sr sv sw th tl tr uk vi zh_CN zh_HK zh_TW zu make[5]: *** [.aapt.deps] Error 1 make[5]: *** Deleting file `.aapt.deps' make[5]: *** Waiting for unfinished jobs....

/build/mobile/android/base/resources/layout-v11/new_tablet_tabs_item_cell.xml:25: error: Error: No resource found that matches the given name (at 'textColor' with value '@color/new_tablet_tab_item_title'). /builds/slave/try-and- Same symptoms as Bug 1093636, Bug 1093394; the precise details don't really matter. I'm guessing this is just the build flag for newtablet being turned off for release builds. Lucas'll be able to confirm.

Grr, yeah, one more instance of a tablet-specific resource missing. Fixing.

Tracking as we prefer clean beta!

RyanVM: can you verify that the try build you pushed is mozilla-central masquerading as mozilla-beta? I can try to understand this locally.

Correct. Comment 1 shows other similar bugs from the past.

Any updates here?

This should be as trivial as fixing this: # Enable the new tablet UI in pre-release builds # if the max Android sdk is undefined or at least 11. if test ! "$RELEASE_BUILD"; then MOZ_ANDROID_NEW_TABLET_UI=1 fi That is: let the new tablet UI ride the trains. Then, or now, we can entirely unconditionalize the feature: Bug 1106935.

Mark, this has been on file for over a month now and blocks me from being able to look for any test bustage before the next beta uplift (which is coming shortly after the new year). Can we please get someone to implement the fixes discussed in the comments here so I can get some test coverage before everybody disappears for the holidays?

Mike - Tablet UI is riding the trains for Fx36. Can we get the code cleaned up enough to remove the conditional parts? Do we have bugs filed for removing the "side by side" nature of the code? Let's make sure Fx36 moves to Beta cleanly

(In reply to Mark Finkle (:mfinkle) from comment #9) > Do we have bugs filed for > removing the "side by side" nature of the code? Yes: as mentioned earlier, Bug 1106935.

This should do the trick.

Comment on attachment 8536642 [details] [diff] [review] Part 0: Allow new tablet UI to ride the trains. v1 This is the bare minimum. I think this patch would allow the "Enable new Tablet UI" setting to ship as well. We don't want that. I am fine with landing this patch, but we must get whatever other "removals" prepped for Fx36 before it merges to Beta.

This kills part of a test, the checkbox, and takes some of the new tablet switching code down to a stub for removal elsewhere. This builds, but I haven't tested it. Michael, could you carry this forward?

As I mentioned in a comment on https://bugzilla.mozilla.org/show_bug.cgi?id=1093394#c7 - I get a portion of this error (about missing resources) when building without the new tablet UI...would that affect this issue?

Nevermind - ignore my last comment. I thought that the suggested fix was to disable the new tablet UI - but in actuality, it is *enabling* the new tablet UI. Sorry.

Comment on attachment 8536692 [details] [diff] [review] Part 1: assume new tablet UI. v1 Review of attachment 8536692 [details] [diff] [review]: ----------------------------------------------------------------- Remove the associated Strings too. ::: mobile/android/base/NewTabletUI.java @@ +16,5 @@ > if (!HardwareUtils.isTablet()) { > return false; > } > > + return true; nit: return HardwareUtils.isTablet(); ::: mobile/android/base/tests/testSettingsMenuItems.java @@ +163,2 @@ > // New tablet UI: we don't allow a page title option. > + settingsMap.get(PATH_DISPLAY).remove(TITLE_BAR_LABEL_ARR); Inside `if (isTablet)`

Taking over for rnewman.

My updates to Richard's patch sans String updates - I realized we don't want those because this will be uplifted.

try run w/ the patch: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b00899638a80 Note that best efforts were made to rebase RyanVM's initial patch (comment 0) onto Aurora for testing this.

try run w/ up-to-date trunk-as-beta patch from RyanVM: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=fabc9cdc499e

testSettingsMenuItems works for me locally on my N7: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=4c69ff8a15c8 Added @RobocopTarget for the other failures.

Update owner info. I'll handle the burning tests in part 2.

I'm not sure why the try push in comment 21 would affect API 9... I wonder if there could be an issue in the split patch?

Comment on attachment 8538018 [details] [diff] [review] Part 1: assume new tablet UI Review of attachment 8538018 [details] [diff] [review]: ----------------------------------------------------------------- Tag - no tag backs!

I can't reproduce the failures in the testAboutPasswords or testGetUserMedia tests locally and I'm having difficulty identifying useful information in the logs - I'm going to try pulling and setting up a new try run to ensure we're still getting the same errors.

Comment on attachment 8538018 [details] [diff] [review] Part 1: assume new tablet UI Review of attachment 8538018 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/NewTabletUI.java @@ +13,3 @@ > public class NewTabletUI { > + public static boolean isEnabled(final Context context) { > + return HardwareUtils.isTablet(); Something to think about: just inline this and eliminate NewTabletUI altogether? Bigger patch, but it kills this class.

(In reply to Richard Newman [:rnewman] from comment #26) > Something to think about: just inline this and eliminate NewTabletUI > altogether? Bigger patch, but it kills this class. If there are no major perf wins to be had here, I'd rather keep it in - it makes it easier to find and update these references for bug 1106935.

(In reply to Michael Comella (:mcomella) from comment #25) > I can't reproduce the failures in the testAboutPasswords or testGetUserMedia > tests locally Actually, now I can - I think it's because the patch requires a clobber.

Seems this test was not disabled on non-Nightly builds though the feature was.

GCP, are all of the features of testGetUserMedia available on all release channels? e.g. tab streaming In my try run [1] for trunk-as-beta simulation, testGetUserMedia fails on 2.3 and doesn't seem to run on 4.0 - I imagine [2] is the reason. Note that testGetUserMedia fails locally. [1]: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=66bbab144ba4 [2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/testGetUserMedia.java?rev=d7a6ecdac6b4#32

That check will pass on try because the test slaves have emulated cameras. You also ran the test on 4.2 x86, not 4.0 ARM, which is why it didn't run. Looking at the try run, it's tab streaming that is failing. We don't have the Google services for Chromecast(?) in the 2.3 APK but I would expect that tab streaming also works without that, just to WebRTC clients? Blassey, can you confirm?

(In reply to Gian-Carlo Pascutto [:gcp] from comment #31) > We don't have > the Google services for Chromecast(?) in the 2.3 APK but I would expect that > tab streaming also works without that, just to WebRTC clients? Blassey, can > you confirm? Yup

Seems tracking protection is not enabled in non-Nightly builds: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/preferences/GeckoPreferences.java?rev=44916c82049c#683

Actually, the previous pt. 4 won't work - this one works locally.

First four patches: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=fd5f176a7258

(In reply to Gian-Carlo Pascutto [:gcp] from comment #31) > You also ran the test on 4.2 x86, not 4.0 ARM, which is why it didn't run. According to the logs, it ran up until the camera check. robocop.ini [1] implies this test is not disabled on x86. It also fails locally on my N7 and N9. The push in comment 35 includes 4.0 - I wonder if it will show the same failures. Any ideas what else this might be? I'll keep investigating in the mean-time. [1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/robocop.ini?rev=64f427666f49#43

Following the String we're looking for, "Choose a tab to stream" [1], it is defined as tabshare.title [2], which is only used in TabSource.js [3]. While I don't know how the JS manifest files work, it appears it may be disabled for non-release builds [4] [5]. Since [3] references creating the prompt that we see, I think that may be why this test is failing. Finkle & GCP, any thoughts? Can I disable this test on release builds? [1]: https://mxr.mozilla.org/mozilla-central/search?string=Choose%20a%20tab%20to%20stream&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central [2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/locales/en-US/chrome/browser.properties?rev=d9052c420ed3#377 [3]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/components/TabSource.js?rev=c474691b9c0f#36 [4]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/components/MobileComponents.manifest?rev=4f270128536a#108 [5]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/installer/package-manifest.in?rev=70fe31e4b65e#347

(In reply to Michael Comella (:mcomella) from comment #37) > Following the String we're looking for, "Choose a tab to stream" [1], it is > defined as tabshare.title [2], which is only used in TabSource.js [3]. While > I don't know how the JS manifest files work, it appears it may be disabled > for non-release builds [4] [5] The opposite: `ifndef RELEASE_BUILD` means that the manifest section will be included (and the code available) if RELEASE_BUILD is *not* defined. If tab sharing should ride the trains, you'll need to remove that conditional, and any siblings it has scattered around the tree. Bug 996358 can be your guide to that.

(In reply to Richard Newman [:rnewman] from comment #38) > The opposite: `ifndef RELEASE_BUILD` means that the manifest section will be > included (and the code available) if RELEASE_BUILD is *not* defined. That is what I intended to say (-_-') meaning... > Can I disable this test on release builds? Confirmed for disable.

Added some hero logging.

Note some additional cleanup in part 4.

Comment on attachment 8541073 [details] [diff] [review] Part 2: Disable testAboutPasswords on non-Nightly builds Review of attachment 8541073 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/tests/testAboutPasswords.java @@ +17,5 @@ > + > + @Override > + public void testJavascript() throws Exception { > + // This feature is only available in Nightly. > + if (!AppConstants.NIGHTLY_BUILD) { Double-check for me: is this feature non-release (Aurora or Nightly), or Nightly only? @@ +21,5 @@ > + if (!AppConstants.NIGHTLY_BUILD) { > + Log.d(LOGTAG, "This test is disabled on non-Nightly builds: returning"); > + return; > + } > + } You're missing a `super.testJavascript()` call here.

(In reply to Richard Newman [:rnewman] from comment #44) > ::: mobile/android/base/tests/testAboutPasswords.java > > + if (!AppConstants.NIGHTLY_BUILD) { > > Double-check for me: is this feature non-release (Aurora or Nightly), or > Nightly only? Nightly only: https://mxr.mozilla.org/mozilla-central/source/mobile/android/components/AboutRedirector.js?rev=1050ea09ba3e#87 > You're missing a `super.testJavascript()` call here. Welp, I feel silly. Fixed.

Comment on attachment 8542197 [details] [diff] [review] Part 2: Disable testAboutPasswords on non-Nightly builds Review of attachment 8542197 [details] [diff] [review]: ----------------------------------------------------------------- r+ if you find the right log tag! ::: mobile/android/base/tests/testAboutPasswords.java @@ +11,2 @@ > public class testAboutPasswords extends JavascriptTest { > + private static final String LOGTAG = testAboutPasswords.class.getSimpleName(); This should really be a value that'll be picked up by the test harness. I *think* this'll be invisible when viewed in the logs, because "Full Logs" doesn't actually show all logcat output. Time to ask gbrown...

Looks like it's not so much logtag filtering that we need to worry about, so much as not reporting the whole log. So let's do something a little different. Add a no-op assertion with the log message instead, so at least it'll show up!

(In reply to Richard Newman [:rnewman] from comment #48) > Looks like it's not so much logtag filtering that we need to worry about, so > much as not reporting the whole log. Details: Filtering: https://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozdevice/mozdevice/devicemanager.py?rev=da9a33e93832#149 https://mxr.mozilla.org/mozilla-central/source/build/mobile/remoteautomation.py?rev=edd717f27f37#20 We only take the logcat at the end of the test run so we only get the last few messages.

Updated w/ mAsserter.dumpLog.

Remove unnecessary Log import. -_-

Log -> mAsserter.dumpLog.

Note: updated patches should not be so different from try run in comment 46.

https://hg.mozilla.org/integration/fx-team/rev/d8fd7efe9ea6 https://hg.mozilla.org/integration/fx-team/rev/30a8e30cf31b https://hg.mozilla.org/integration/fx-team/rev/a32868881bc7 https://hg.mozilla.org/integration/fx-team/rev/92e3d07b6721 https://hg.mozilla.org/integration/fx-team/rev/3e5f3d222e21

Note: the try build from comment 46 uses a trunk-as-beta simulation patch - I forgot to push against trunk on try, but I don't expect any issues.

NI to uplift to 36. (Aurora)

https://hg.mozilla.org/mozilla-central/rev/d8fd7efe9ea6 https://hg.mozilla.org/mozilla-central/rev/30a8e30cf31b https://hg.mozilla.org/mozilla-central/rev/a32868881bc7 https://hg.mozilla.org/mozilla-central/rev/92e3d07b6721 https://hg.mozilla.org/mozilla-central/rev/3e5f3d222e21

Comment on attachment 8536642 [details] [diff] [review] Part 0: Allow new tablet UI to ride the trains. v1 Request applies to parts 0 - 4. Approval Request Comment [Feature/regressing bug #]: New tablet UI, FF 36, bug 1014156 [User impact if declined]: The new tablet UI will not be enabled by default in FF 36, nor will it be available to toggle on. The tree will burn due to inconsistent test state with regards to the "RELEASE_BUILD" flag. [Describe test coverage new/current, TBPL]: This patch fixes the existing tests which cover most of the new tablet functionality but there are no new tests to cover the tab strip - bug 1064828. [Risks and why]: There may still be mismatched release build and new tablet flags, causing an inconsistent/non-building browser build, but this is unlikely given the trunk-as-beta build I tested with. More simply, these patches may not completely fix the issue in this bug. [String/UUID change made/needed]: N/A

https://hg.mozilla.org/releases/mozilla-aurora/rev/aaa8e2cd46e4 https://hg.mozilla.org/releases/mozilla-aurora/rev/ce13de5c49b4 https://hg.mozilla.org/releases/mozilla-aurora/rev/2c0fdd936e83 https://hg.mozilla.org/releases/mozilla-aurora/rev/db56e49069f4 Part 2 was skipped because testAboutPasswords is only on Gecko 37.

Verified green on my latest Try runs. You have earned your gold star, sir! Thanks for hammering this out :)