bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[Eli Zaretskii]

> Date: Mon, 22 Sep 2014 19:43:43 +0200
> From: martin rudalics <rudalics@gmx.at>
> 
>  >    (gdb) p f->text_cols
>  >    $6 = -3  <<<<<<<<<<<<<<<<<<<
> 
> What is the value of f->text_width here?

18, as expected (minimum width is 2 columns).

>  >      (We also don't check errors returned by
>  >      GetClientRect.)
> 
> Should we?

I certainly think so.  If GetClientRect fails, how does it make sense
to use what we find in the rectangle data structure we passed to it?
The values there are just garbage.

> I wonder already why
> 
>         if (f && !FRAME_ICONIFIED_P (f) && msg.msg.wParam != SIZE_MINIMIZED)
> 
> in w32_read_socket didn't catch this.

I have no idea, perhaps because we already made the frame visible, but
the window manager didn't yet have time to actually do so.  The fact
is we do get all-zero rectangle, I've seen that in the debugger.

>  > +        if (GetClientRect (msg.msg.hwnd, &rect)
>  > +            /* GetClientRect evidently returns (0, 0, 0, 0) if
>  > +               called on a minimized frame.  Such "dimensions"
>  > +               aren't useful anyway.  */
>  > +            && !(rect.bottom == 0
>  > +                 && rect.top == 0
>  > +                 && rect.left == 0
>  > +                 && rect.right == 0))
> 
> It certainly can't harm to do that but I doubt whether it's worth to
> include a similar change for the other platforms.

I don't suggest such changes for other platforms, only for w32.

>  > +  /* Recompute the dimensions in character units, since
>  > +     check_frame_size might have changed the pixel dimensions.  */
>  > +  /* Consider rounding here: Currently, the root window can be
>  > +     larger than the frame in terms of columns/lines.  */
>  > +  new_cols = new_text_width / FRAME_COLUMN_WIDTH (f);
>  > +  new_lines = new_text_height / FRAME_LINE_HEIGHT (f);
> 
> This part should fix the problem for all platforms.
> 
> Please check it in but please also make sure that only the changes in
> adjust_decode_mode_spec_buffer and maybe those of w32_read_socket get
> propagated to the trunk.

The rest of the changes won't get propagated because the relevant code
in change_frame_size_1 is gone on the trunk.

> Did you verify that the trunk handles your .emacs.desktop correctly?

No, I didn't have a trunk binary on that machine.  But if the
validation of the frame dimensions on the tyrunk doesn't suffer from
this problem, the bug shouldn't happen.

Btw, the problem has nothing to do with my .emacs.desktop: the call to
w32_read_socket was from the loop where we wait for the frame to
become visible.  It can happen at any time with any desktop file that
re-creates more than one frame.