Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(316)

Issue 17502005: Revert r207560 and r207566 to reland r207511. (Closed)

Created:
7 years, 6 months ago by Daniel Erat
Modified:
7 years, 6 months ago
Reviewers:
bshe
CC:
chromium-reviews, sadrul, nkostylev+watch_chromium.org, ben+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, oshima
Visibility:
Public.

Description

Revert r207560 and r207566 to reland r207511. The win_aura builder appeared to have trouble running animations in DesktopBackgroundControllerTest.SetDefaultWallpaper; this version of the change splits that test into several smaller tests so that animations can be avoided. Original change: ash: Add flags for specifying default wallpaper. This adds new command-line flags for specifying image files that will be used as default wallpaper: --ash-default-guest-wallpaper-large --ash-default-guest-wallpaper-small --ash-default-wallpaper-large --ash-default-wallpaper-small This makes it easier for Chrome OS to have per-device custom wallpaper. (We previously loaded the default wallpaper from a resource file, but that prevents the same build of Chrome from being used on different devices.) BUG=248764 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207800

Patch Set 1 #

Total comments: 2

Patch Set 2 : break up test to avoid animations #

Patch Set 3 : use SupportsMultipleDisplays() #

Patch Set 4 : add more SupportsMultipleDisplays checks #

Unified diffs Side-by-side diffs Delta from patch set Stats (+662 lines, -323 lines) Patch
M ash/ash_switches.h View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M ash/ash_switches.cc View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
M ash/desktop_background/desktop_background_controller.h View 8 chunks +42 lines, -27 lines 0 comments Download
M ash/desktop_background/desktop_background_controller.cc View 12 chunks +149 lines, -68 lines 0 comments Download
M ash/desktop_background/desktop_background_controller_unittest.cc View 1 2 3 10 chunks +312 lines, -27 lines 0 comments Download
M ash/desktop_background/wallpaper_resizer.h View 3 chunks +19 lines, -11 lines 0 comments Download
M ash/desktop_background/wallpaper_resizer.cc View 1 3 chunks +63 lines, -59 lines 0 comments Download
M ash/desktop_background/wallpaper_resizer_unittest.cc View 5 chunks +24 lines, -22 lines 0 comments Download
M ash/display/display_manager.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M ash/shell/content_client/shell_browser_main_parts.cc View 1 chunk +2 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/login/chrome_restart_request.cc View 1 2 1 chunk +7 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/login/wallpaper_manager.cc View 1 chunk +3 lines, -18 lines 0 comments Download
M chrome/browser/chromeos/login/wallpaper_manager_browsertest.cc View 11 chunks +25 lines, -82 lines 0 comments Download

Messages

Total messages: 14 (0 generated)
Daniel Erat
7 years, 6 months ago (2013-06-20 23:15:14 UTC) #1
bshe
On 2013/06/20 23:15:14, Daniel Erat wrote: lgtm
7 years, 6 months ago (2013-06-20 23:19:32 UTC) #2
Daniel Erat
jamescook@ did some Windows builds of ash_unittests on my behalf. We verified that we could ...
7 years, 6 months ago (2013-06-20 23:24:56 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/derat@chromium.org/17502005/3001
7 years, 6 months ago (2013-06-20 23:26:44 UTC) #4
bshe
https://codereview.chromium.org/17502005/diff/1/ash/desktop_background/desktop_background_controller_unittest.cc File ash/desktop_background/desktop_background_controller_unittest.cc (right): https://codereview.chromium.org/17502005/diff/1/ash/desktop_background/desktop_background_controller_unittest.cc#newcode416 ash/desktop_background/desktop_background_controller_unittest.cc:416: observer.WaitForWallpaperDataChanged(); Your new test if still good. For the ...
7 years, 6 months ago (2013-06-21 00:43:14 UTC) #5
Daniel Erat
https://codereview.chromium.org/17502005/diff/1/ash/desktop_background/desktop_background_controller_unittest.cc File ash/desktop_background/desktop_background_controller_unittest.cc (right): https://codereview.chromium.org/17502005/diff/1/ash/desktop_background/desktop_background_controller_unittest.cc#newcode416 ash/desktop_background/desktop_background_controller_unittest.cc:416: observer.WaitForWallpaperDataChanged(); On 2013/06/21 00:43:14, bshe wrote: > Your new ...
7 years, 6 months ago (2013-06-21 00:53:31 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/derat@chromium.org/17502005/27001
7 years, 6 months ago (2013-06-21 00:55:36 UTC) #7
Daniel Erat
I'm still seeing failures on win8_aura (while win7_aura is fine). This time, it's returning the ...
7 years, 6 months ago (2013-06-21 05:11:08 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/derat@chromium.org/17502005/42001
7 years, 6 months ago (2013-06-21 05:11:24 UTC) #9
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 6 months ago (2013-06-21 10:49:04 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/derat@chromium.org/17502005/42001
7 years, 6 months ago (2013-06-21 11:48:05 UTC) #11
commit-bot: I haz the power
Change committed as 207800
7 years, 6 months ago (2013-06-21 11:54:01 UTC) #12
oshima
On win8, most tests that rely on real host window size fails because you can't ...
7 years, 6 months ago (2013-06-21 14:20:33 UTC) #13
Daniel Erat
7 years, 6 months ago (2013-06-21 14:22:37 UTC) #14
I guess that the reason that this wasn't a problem before was that the
tests that were exercising
DesktopBackgroundController::GetAppropriateResolution() were in
chrome/browser/chromeos/login instead of part of
DesktopBackgroundControllerTest. :-(


On Fri, Jun 21, 2013 at 7:20 AM, oshima <oshima@chromium.org> wrote:

> On win8, most tests that rely on real host window size fails because
> you can't change host window size on win8/metro. We probably
> should add similar method like SupportsHostWindowResize()
> to skip such tests on win8 bots.
>
>
>
> On Thu, Jun 20, 2013 at 10:11 PM, <derat@chromium.org> wrote:
>
>> I'm still seeing failures on win8_aura (while win7_aura is fine). This
>> time,
>> it's returning the small wallpaper in places where it should be using the
>> large
>> wallpaper. I suspect that UpdateDisplay() is broken on Windows 8 even in
>> the
>> single-display case and the displays aren't getting created as requested.
>>
>> I feel like I've spent too much time trying to work around testing bugs
>> on a
>> platform that we don't care about with regard to this change, so I've
>> added
>> SupportsMultipleDisplays() calls to the beginning of all of these tests
>> to make
>> them not run on platforms where the testing framework seems to be buggy.
>> We'll
>> still have testing coverage on win7_aura, along with all of the Chrome OS
>> builders that we actually care about.
>>
>>
https://codereview.chromium.**org/17502005/<https://codereview.chromium.org/1...
>>
>
>

Powered by Google App Engine
This is Rietveld 408576698