<AWT Dev> Code Review Request for CR 7024749 - JDK7 b131---a crash in: Java_sun_awt_windows_ThemeReader_isGetThemeTransitionDurationDefined+0x75

[Artem Ananiev]

Hi, Oleg,

I'm not the expert in this area, it would be fine if Dmitry or Naoto 
could also take a look. Anyway, here are some comments from my side:

1. I see no reason to change ImmGetContext() to ImmGetHWnd(). Everywhere 
in the code ImmGetHWnd() is followed by ImmGetContext(), and these two 
calls can easily be combined into a single method in AwtComponent.

2. Comment about focus proxy in AwtComponent::OpenCandidateWindow() is 
now obsolete. Instead, you need to add a comment why we send 
WM_IME_NOTIFY to this component (GetHWnd()), not to its focus proxy.

Minor comments:

3. There is no need to call ImmReleaseContext() if hIMC is NULL in 
AwtComponent::WmImeSetContext().

4. In WM_ACTIVATE handler, I would first handle WM_IME_ENDCOMPOSITION, 
then call ImmReleaseContext().

5. In the same WM_ACTIVATE handler, what's the reason of direct call to 
DefWindowProc() instead of regular ::SendMessage()?

Thanks,

Artem

On 4/19/2012 5:41 PM, Oleg Pekhovskiy wrote:
> Hi!
>
> Please review the fix for this CR:
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7024749
>
> Webrev:
> http://cr.openjdk.java.net/~bagiras/8/7024749.1
>
> The idea of this fix based on IMM interaction theory (from MSDN) and
> actual implementation in AWT.
>
> AWT creates its own IME context. It is shared between all AWT windows.
> This context is associated with window each time it receives WM_ACTIVATE
> message (which surplus but not critical).
> WM_IME_SETCONTEXT and WM_IME_NOTIFY (with INM_OPENSTATUSWINDOW) messages
> apply to all AWT windows and usually are sent to the top-level window.
> So there is no need to send them to the focus proxy.
> That's why I removed CallProxyDefWindowProc() from their handlers in
> AwtComponent::WindowProc().
> I also removed switch cases for those messages from
> AwtFrame::ProxyWindowProc() because they didn't make any sense there.
> Thus, only IME messages between WM_IME_STARTCOMPOSITION and
> WM_IME_ENDCOMPOSITION are sent to the proxy focus owner - that's a
> typing period.
>
> I changed all places where IME context was used according to MSDN notes:
> http://msdn.microsoft.com/en-us/library/windows/desktop/dd318639(v=vs.85).aspx
>
> "The application retrieves the handle by using the ImmGetContext
> function. It can use the retrieved handle in subsequent calls to the IMM
> functions
> to retrieve and set IME values, such as the composition window style,
> the composition style, and the status window position.
> Once the application has finished using the context, it must release the
> context using the ImmReleaseContext function."
>
> All changes were tested by the regression tests in 'test/java/awt/Focus'
> and all JCK tests.
> I also tested them on hierarchy of owned windows with different
> enabled/disabled states.
> No bad influence was found. From that point of view it is safe.
>
> Thanks,
> Oleg