New issue
Advanced search Search tips

Issue 646342 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

chrome_public_test_apk failing on 3 builders in ContextualSearchManagerTest

Project Member Reported by aber...@chromium.org, Sep 13 2016

Issue description

chrome_public_test_apk failing on 3 builders

Type: build-failure

Builders failed on: 
- KitKat Tablet Tester: 
  https://build.chromium.org/p/chromium.android/builders/KitKat%20Tablet%20Tester
- Marshmallow Tablet Tester: 
  https://build.chromium.org/p/chromium.android/builders/Marshmallow%20Tablet%20Tester
- Lollipop Tablet Tester:
  https://build.chromium.org/i/chromium.android/builders/Lollipop%20Tablet%20Tester

Failing tests:
org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest#testChainedSearchCreatesNewContent 
and 
org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest#testChainedTapsRemovedFromHistory 

Log of failure:

I  911.589s Main  2 failed tests remain.
C  911.596s Main  ********************************************************************************
C  911.596s Main  Detailed Logs
C  911.596s Main  ********************************************************************************
C  911.600s Main  [FAIL] org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest#testChainedSearchCreatesNewContent:
C  911.600s Main  junit.framework.AssertionFailedError: Values did not match. Expected: Resolution, actual: null
C  911.600s Main  	at org.chromium.content.browser.test.util.CriteriaHelper.pollInstrumentationThread(CriteriaHelper.java:74)
C  911.600s Main  	at org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest.waitForSelectionToBe(ContextualSearchManagerTest.java:179)
C  911.600s Main  	at org.chromium.chrome.browser.contextualsearch.ContextualSearchFakeServer$FakeTapSearch.simulate(ContextualSearchFakeServer.java:195)
C  911.600s Main  	at org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest.simulateTapSearch(ContextualSearchManagerTest.java:261)
C  911.600s Main  	at org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest.testChainedSearchCreatesNewContent(ContextualSearchManagerTest.java:2317)
C  911.600s Main  	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
C  911.600s Main  	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
C  911.600s Main  	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192)
C  911.600s Main  	at org.chromium.chrome.test.ChromeActivityTestCaseBase.runTest(ChromeActivityTestCaseBase.java:726)
C  911.600s Main  	at org.chromium.base.test.BaseTestResult.runParameterized(BaseTestResult.java:161)
C  911.600s Main  	at org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:124)
C  911.600s Main  	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191)
C  911.600s Main  	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176)
C  911.600s Main  	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555)
C  911.600s Main  	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1853)
C  911.600s Main  
C  911.601s Main  [FAIL] org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest#testChainedTapsRemovedFromHistory:
C  911.601s Main  junit.framework.AssertionFailedError: Values did not match. Expected: Resolution, actual: null
C  911.601s Main  	at org.chromium.content.browser.test.util.CriteriaHelper.pollInstrumentationThread(CriteriaHelper.java:74)
C  911.601s Main  	at org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest.waitForSelectionToBe(ContextualSearchManagerTest.java:179)
C  911.601s Main  	at org.chromium.chrome.browser.contextualsearch.ContextualSearchFakeServer$FakeTapSearch.simulate(ContextualSearchFakeServer.java:195)
C  911.601s Main  	at org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest.simulateTapSearch(ContextualSearchManagerTest.java:261)
C  911.601s Main  	at org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest.testChainedTapsRemovedFromHistory(ContextualSearchManagerTest.java:2478)
C  911.601s Main  	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
C  911.601s Main  	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
C  911.601s Main  	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192)
C  911.601s Main  	at org.chromium.chrome.test.ChromeActivityTestCaseBase.runTest(ChromeActivityTestCaseBase.java:726)
C  911.601s Main  	at org.chromium.base.test.BaseTestResult.runParameterized(BaseTestResult.java:161)
C  911.601s Main  	at org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:124)
C  911.601s Main  	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191)
C  911.601s Main  	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176)
C  911.601s Main  	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555)
C  911.602s Main  	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1853)
C  911.602s Main  ********************************************************************************


 
Labels: OS-Android Pri-1 Type-Bug-Regression
Summary: chrome_public_test_apk failing on 3 builders in ContextualSearchManagerTest (was: chrome_public_test_apk failing on 2 builders in ContextualSearchManagerTest)
Cc: pdr@chromium.org
Status: Assigned (was: Available)
From a bisect, and confirmed by a local revert, this seems to be caused by:

commit eccac1fd6db85731fb95b11915fc2197a6ceb0c3
Author: pdr <pdr@chromium.org>
Date:   Mon Sep 12 22:13:49 2016 -0700

    Do not let text-size-adjust override accessibility font settings
    
    The text autosizer is used for both making desktop pages more legible
    on mobile, and for applying the accessibility font scale factor. When
    text-size-adjust support was added, pages were able to override the
    accessibility setting in addition to the autosizing multiplier.
    
    This patch ensures accessibility font scale settings are respected even
    with text-size-adjust. A followup bug has been filed (crbug.com/645717)
    for moving the accessibility font scale factor out of the autosizer.
    
    This patch also re-enables support for text-size-adjust which was
    disabled temporarily due to breaking accessibility settings.
    
    BUG= 645269 
    
    Review-Url: https://codereview.chromium.org/2329733002
    Cr-Commit-Position: refs/heads/master@{#418173}

I will revert this on master, although it is not obvious to me what the relationship is.
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ec856cdfb31f023a832547f4f702858e9b8a903

commit 3ec856cdfb31f023a832547f4f702858e9b8a903
Author: aberent <aberent@chromium.org>
Date: Tue Sep 13 17:05:51 2016

Revert of Do not let text-size-adjust override accessibility font settings (patchset #2 id:20001 of https://codereview.chromium.org/2329733002/ )

Reason for revert:
Broke ContextualSearchManagerTest#testChainedSearchCreatesNewContent and ContextualSearchManagerTest#testChainedTapsRemovedFromHistory

BUG= 646342 

Original issue's description:
> Do not let text-size-adjust override accessibility font settings
>
> The text autosizer is used for both making desktop pages more legible
> on mobile, and for applying the accessibility font scale factor. When
> text-size-adjust support was added, pages were able to override the
> accessibility setting in addition to the autosizing multiplier.
>
> This patch ensures accessibility font scale settings are respected even
> with text-size-adjust. A followup bug has been filed (crbug.com/645717)
> for moving the accessibility font scale factor out of the autosizer.
>
> This patch also re-enables support for text-size-adjust which was
> disabled temporarily due to breaking accessibility settings.
>
> BUG= 645269 
>
> Committed: https://crrev.com/eccac1fd6db85731fb95b11915fc2197a6ceb0c3
> Cr-Commit-Position: refs/heads/master@{#418173}

TBR=skobes@chromium.org,pdr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 645269 

Review-Url: https://codereview.chromium.org/2335193003
Cr-Commit-Position: refs/heads/master@{#418278}

[modify] https://crrev.com/3ec856cdfb31f023a832547f4f702858e9b8a903/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
[modify] https://crrev.com/3ec856cdfb31f023a832547f4f702858e9b8a903/third_party/WebKit/Source/core/layout/TextAutosizer.h
[modify] https://crrev.com/3ec856cdfb31f023a832547f4f702858e9b8a903/third_party/WebKit/Source/core/layout/TextAutosizerTest.cpp

Cc: -pdr@chromium.org pedrosim...@chromium.org
Owner: pdr@chromium.org
Assigning to pdr@ as the owner of the reverted CL.
+pedrosimonetti@ as the author of the tests (I believe).
Cc: -pedrosim...@chromium.org donnd@chromium.org

Comment 7 by donnd@chromium.org, Sep 13 2016

Components: UI>Browser>Mobile>TouchToSearch

Comment 8 by pdr@chromium.org, Sep 14 2016

Cc: skobes@chromium.org
Components: -UI>Browser>Mobile>TouchToSearch Blink>TextAutosize
Thanks for the rollout. This was in fact a typo in my patch.

I'm relanding in https://codereview.chromium.org/2340553003 with a test that has coverage on all bots. There are no trybots for the configurations that initially failed so I'll be watching the builders as this lands.
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1229b1414d64c314666d2fb3bf47706f2b5e947b

commit 1229b1414d64c314666d2fb3bf47706f2b5e947b
Author: pdr <pdr@chromium.org>
Date: Wed Sep 14 04:40:53 2016

(reland) Do not let text-size-adjust override accessibility font settings

The text autosizer is used for both making desktop pages more legible
on mobile, and for applying the accessibility font scale factor. When
text-size-adjust support was added, pages were able to override the
accessibility setting in addition to the autosizing multiplier.

This patch ensures accessibility font scale settings are respected even
with text-size-adjust. A followup bug has been filed (crbug.com/645717)
for moving the accessibility font scale factor out of the autosizer.

This patch also re-enables support for text-size-adjust which was
disabled temporarily due to breaking accessibility settings.

This was reverted due to a forgotten "!" in checking whether the
viewport was specified by the author in TextAutosizer.cpp. A new test
has been added to catch this in the future. The unneeded meta viewport
declarations in all other tests have been removed because they had
no effect with the meta viewport setting disabled.

Original review:
https://codereview.chromium.org/2329733002

BUG= 645269 , 646342 

Review-Url: https://codereview.chromium.org/2340553003
Cr-Commit-Position: refs/heads/master@{#418488}

[modify] https://crrev.com/1229b1414d64c314666d2fb3bf47706f2b5e947b/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
[modify] https://crrev.com/1229b1414d64c314666d2fb3bf47706f2b5e947b/third_party/WebKit/Source/core/layout/TextAutosizer.h
[modify] https://crrev.com/1229b1414d64c314666d2fb3bf47706f2b5e947b/third_party/WebKit/Source/core/layout/TextAutosizerTest.cpp

Comment 10 by pdr@chromium.org, Sep 14 2016

Status: Fixed (was: Assigned)
The bots now look happy, the engineers look as happy as engineers can, closing.
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 20 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d25ac697753f02ac5c923d65402d75b4211cab28

commit d25ac697753f02ac5c923d65402d75b4211cab28
Author: pdr@chromium.org <pdr@chromium.org>
Date: Tue Sep 20 17:59:25 2016

(reland) Do not let text-size-adjust override accessibility font settings

The text autosizer is used for both making desktop pages more legible
on mobile, and for applying the accessibility font scale factor. When
text-size-adjust support was added, pages were able to override the
accessibility setting in addition to the autosizing multiplier.

This patch ensures accessibility font scale settings are respected even
with text-size-adjust. A followup bug has been filed (crbug.com/645717)
for moving the accessibility font scale factor out of the autosizer.

This patch also re-enables support for text-size-adjust which was
disabled temporarily due to breaking accessibility settings.

This was reverted due to a forgotten "!" in checking whether the
viewport was specified by the author in TextAutosizer.cpp. A new test
has been added to catch this in the future. The unneeded meta viewport
declarations in all other tests have been removed because they had
no effect with the meta viewport setting disabled.

Original review:
https://codereview.chromium.org/2329733002

BUG= 645269 , 646342 

Review-Url: https://codereview.chromium.org/2340553003
Cr-Commit-Position: refs/heads/master@{#418488}
(cherry picked from commit 1229b1414d64c314666d2fb3bf47706f2b5e947b)

Review URL: https://codereview.chromium.org/2355053002 .

Cr-Commit-Position: refs/branch-heads/2840@{#444}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/d25ac697753f02ac5c923d65402d75b4211cab28/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
[modify] https://crrev.com/d25ac697753f02ac5c923d65402d75b4211cab28/third_party/WebKit/Source/core/layout/TextAutosizer.h
[modify] https://crrev.com/d25ac697753f02ac5c923d65402d75b4211cab28/third_party/WebKit/Source/core/layout/TextAutosizerTest.cpp

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d25ac697753f02ac5c923d65402d75b4211cab28

commit d25ac697753f02ac5c923d65402d75b4211cab28
Author: pdr@chromium.org <pdr@chromium.org>
Date: Tue Sep 20 17:59:25 2016

(reland) Do not let text-size-adjust override accessibility font settings

The text autosizer is used for both making desktop pages more legible
on mobile, and for applying the accessibility font scale factor. When
text-size-adjust support was added, pages were able to override the
accessibility setting in addition to the autosizing multiplier.

This patch ensures accessibility font scale settings are respected even
with text-size-adjust. A followup bug has been filed (crbug.com/645717)
for moving the accessibility font scale factor out of the autosizer.

This patch also re-enables support for text-size-adjust which was
disabled temporarily due to breaking accessibility settings.

This was reverted due to a forgotten "!" in checking whether the
viewport was specified by the author in TextAutosizer.cpp. A new test
has been added to catch this in the future. The unneeded meta viewport
declarations in all other tests have been removed because they had
no effect with the meta viewport setting disabled.

Original review:
https://codereview.chromium.org/2329733002

BUG= 645269 , 646342 

Review-Url: https://codereview.chromium.org/2340553003
Cr-Commit-Position: refs/heads/master@{#418488}
(cherry picked from commit 1229b1414d64c314666d2fb3bf47706f2b5e947b)

Review URL: https://codereview.chromium.org/2355053002 .

Cr-Commit-Position: refs/branch-heads/2840@{#444}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/d25ac697753f02ac5c923d65402d75b4211cab28/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
[modify] https://crrev.com/d25ac697753f02ac5c923d65402d75b4211cab28/third_party/WebKit/Source/core/layout/TextAutosizer.h
[modify] https://crrev.com/d25ac697753f02ac5c923d65402d75b4211cab28/third_party/WebKit/Source/core/layout/TextAutosizerTest.cpp

Sign in to add a comment