Minimize black screen during the boot with multi displays

Minimize black screen during the boot with multi displays
 * Do not try to install Background when a root window is
   added if a background isn't set yet.
 * Set the boot screen background color to the root window
   during the boot if there is external display.

BUG=171050
TEST=manual


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=179502

[oshima, 2013-01-25 21:22:52]

bshe -> desktop_background
marchue -> output_configurator change
derat -> everything

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
File ash/desktop_background/desktop_background_controller.cc (left):

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
ash/desktop_background/desktop_background_controller.cc:350: default: {
removing default as desktop_background_mode_ is enum.

https://codereview.chromium.org/12079002/diff/1/ash/display/display_change_ob...
File ash/display/display_change_observer_x11.cc (right):

https://codereview.chromium.org/12079002/diff/1/ash/display/display_change_ob...
ash/display/display_change_observer_x11.cc:97: XRRUpdateConfiguration(event);
according to http://www.x.org/archive/current/doc/man/man3/Xrandr.3.xhtml,

we're supposed to call this.

[bshe, 2013-01-25 21:46:34]

desktop_background lgtm
Thanks for fixing it.

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
File ash/desktop_background/desktop_background_controller.cc (right):

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
ash/desktop_background/desktop_background_controller.cc:183: // The background
hasn't set yet.
Just curiosity, if we skip the installation here, when do we install the
background widget on the second screen? Would this function get called twice?

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
ash/desktop_background/desktop_background_controller.cc:354: case
BACKGROUND_NONE:
nit: Could you wrap the following with {} to be consistent?

[oshima, 2013-01-25 21:51:49]

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
File ash/desktop_background/desktop_background_controller.cc (right):

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
ash/desktop_background/desktop_background_controller.cc:183: // The background
hasn't set yet.
On 2013/01/25 21:46:34, bshe wrote:
> Just curiosity, if we skip the installation here, when do we install the
> background widget on the second screen? Would this function get called twice?

This can be called during the boot, before the background is set
by startup process. The background will set after all root windows are created.

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
ash/desktop_background/desktop_background_controller.cc:354: case
BACKGROUND_NONE:
On 2013/01/25 21:46:34, bshe wrote:
> nit: Could you wrap the following with {} to be consistent?

{} is necessary when you create variables, which doesn't apply here. I don't
think we need {} just for consistency,
but I can be convinced either way. Dan, WDTY?

[Daniel Erat, 2013-01-26 15:52:30]

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
File ash/desktop_background/desktop_background_controller.cc (right):

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
ash/desktop_background/desktop_background_controller.cc:183: // The background
hasn't set yet.
nit: s/hasn't set/hasn't been set/

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
ash/desktop_background/desktop_background_controller.cc:354: case
BACKGROUND_NONE:
On 2013/01/25 21:51:49, oshima wrote:
> On 2013/01/25 21:46:34, bshe wrote:
> > nit: Could you wrap the following with {} to be consistent?
> 
> {} is necessary when you create variables, which doesn't apply here. I don't
> think we need {} just for consistency,
> but I can be convinced either way. Dan, WDTY?

I'd vote for leaving out the curly brackets.

https://codereview.chromium.org/12079002/diff/1/chromeos/display/output_confi...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/12079002/diff/1/chromeos/display/output_confi...
chromeos/display/output_configurator.cc:569: char chromeos_backcolor[] =
"#FEFEFE";
don't hardcode this here; i think it's defined as a constant somewhere else
(maybe under ui, though... it'd still be best to keep it in a single place so
it'll be easy to change later)

[oshima, 2013-01-26 23:10:42]

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
File ash/desktop_background/desktop_background_controller.cc (right):

https://codereview.chromium.org/12079002/diff/1/ash/desktop_background/deskto...
ash/desktop_background/desktop_background_controller.cc:183: // The background
hasn't set yet.
On 2013/01/26 15:52:30, Daniel Erat wrote:
> nit: s/hasn't set/hasn't been set/

Done.

https://codereview.chromium.org/12079002/diff/1/chromeos/display/output_confi...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/12079002/diff/1/chromeos/display/output_confi...
chromeos/display/output_configurator.cc:569: char chromeos_backcolor[] =
"#FEFEFE";
On 2013/01/26 15:52:30, Daniel Erat wrote:
> don't hardcode this here; i think it's defined as a constant somewhere else
> (maybe under ui, though... it'd still be best to keep it in a single place so
> it'll be easy to change later)

I couldn't find one for sharing, but found this one instead.

const SkColor kChromeOsBootColor = SkColorSetARGB(0xff, 0xfe, 0xfe, 0xfe);

I moved it to ash_constants.h.

https://codereview.chromium.org/12079002/diff/7002/chromeos/display/output_co...
File chromeos/display/output_configurator.h (right):

https://codereview.chromium.org/12079002/diff/7002/chromeos/display/output_co...
chromeos/display/output_configurator.h:77: void Init(bool
is_panel_fitting_enabled, uint32 background_color_argb);
I'm using uint32 for two reasons:
1) chromeos/ doesn't have dependency to skia (yet)
2) I thought about using XColor, but it's anonymous
  struct and I couldn't forward declare it.

Let me know if you have better idea.

[Daniel Erat, 2013-01-27 01:27:23]

lgtm

https://codereview.chromium.org/12079002/diff/11001/chromeos/display/output_c...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/12079002/diff/11001/chromeos/display/output_c...
chromeos/display/output_configurator.cc:569: // XColor uses 16bit per color.
nit: s/16bit/16 bits/

[oshima, 2013-01-27 04:37:10]

https://codereview.chromium.org/12079002/diff/11001/chromeos/display/output_c...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/12079002/diff/11001/chromeos/display/output_c...
chromeos/display/output_configurator.cc:569: // XColor uses 16bit per color.
On 2013/01/27 01:27:23, Daniel Erat wrote:
> nit: s/16bit/16 bits/

Done.

[marcheu, 2013-01-29 04:31:44]

lgtm

[commit-bot: I haz the power, 2013-01-29 19:46:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/12079002/16001

[commit-bot: I haz the power, 2013-01-29 19:46:09]

Failed to apply patch for chromeos/display/output_configurator.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chromeos/display/output_configurator.cc
  Hunk #1 FAILED at 534.
  Hunk #2 succeeded at 557 (offset 6 lines).
  Hunk #3 succeeded at 612 with fuzz 2 (offset 6 lines).
  1 out of 3 hunks FAILED -- saving rejects to file
chromeos/display/output_configurator.cc.rej

Patch:       chromeos/display/output_configurator.cc
Index: chromeos/display/output_configurator.cc
diff --git a/chromeos/display/output_configurator.cc
b/chromeos/display/output_configurator.cc
index
ddfb003c0637a10023b75eab5316d7e61cffe89d..a1ae3320dcf1681dad857efa4363ba4db07e2f2f
100644
--- a/chromeos/display/output_configurator.cc
+++ b/chromeos/display/output_configurator.cc
@@ -534,10 +534,14 @@ OutputConfigurator::OutputConfigurator()
       last_enter_state_time_() {
 }
 
-void OutputConfigurator::Init(bool is_panel_fitting_enabled) {
+OutputConfigurator::~OutputConfigurator() {
+  RecordPreviousStateUMA();
+}
+
+void OutputConfigurator::Init(bool is_panel_fitting_enabled,
+                              uint32 background_color_argb) {
   if (!is_running_on_chrome_os_)
     return;
-
   is_panel_fitting_enabled_ = is_panel_fitting_enabled;
 
   // Cache the initial output state.
@@ -551,6 +555,27 @@ void OutputConfigurator::Init(bool
is_panel_fitting_enabled) {
   // Detect our initial state.
   std::vector<OutputSnapshot> outputs = GetDualOutputs(display, screen);
   connected_output_count_ = outputs.size();
+  if (outputs.size() > 1 && background_color_argb) {
+    // Configuring CRTCs/Framebuffer clears the boot screen image.
+    // Set the same background color while configuring the
+    // display to minimize the duration of black screen at boot
+    // time. The background is filled with black later in
+    // ash::DisplayManager.
+    // crbug.com/171050.
+    XSetWindowAttributes swa = {0};
+    XColor color;
+    Colormap colormap = DefaultColormap(display, 0);
+    // XColor uses 16 bits per color.
+    color.red = (background_color_argb & 0x00FF0000) >> 8;
+    color.green = (background_color_argb & 0x0000FF00);
+    color.blue = (background_color_argb & 0x000000FF) << 8;
+    color.flags = DoRed | DoGreen | DoBlue;
+    XAllocColor(display, colormap, &color);
+    swa.background_pixel = color.pixel;
+    XChangeWindowAttributes(display, window, CWBackPixel, &swa);
+    XFreeColors(display, colormap, &color.pixel, 1, 0);
+  }
+
   output_state_ = InferCurrentState(display, screen, outputs);
   // Ensure that we are in a supported state with all connected displays
powered
   // on.
@@ -585,10 +610,6 @@ void OutputConfigurator::Init(bool
is_panel_fitting_enabled) {
       SetIsProjecting(is_projecting);
 }
 
-OutputConfigurator::~OutputConfigurator() {
-  RecordPreviousStateUMA();
-}
-
 bool OutputConfigurator::CycleDisplayMode() {
   VLOG(1) << "CycleDisplayMode";
   if (!is_running_on_chrome_os_)

[commit-bot: I haz the power, 2013-01-29 21:25:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/12079002/15011

[commit-bot: I haz the power, 2013-01-30 01:47:49]

Change committed as 179502