Fix incorrect focus cycling issues in ChromeFrame full tab mode. This happens...

Fix incorrect focus cycling issues in ChromeFrame full tab mode. This happens whenever a tab rendering a chrome frame
page receives focus by switching to it. Whenever we receive focus in ChromeFrame we send over an IPC to set the initial
focus to Chrome. In this IPc we invoke the FocusThroughTabTraversal function on the TabContents which basically cycles
through the focus on the page which is not correct for full tab mode.

Fix is to handle the WM_SETFOCUS message in the active document and invoke the GiveFocusToChrome with false indicating
that we don't want to invoke the FocusThroughTabTraversal function which cycles through the view. It is sufficient to
set focus to the tab.

We also handle the WM_SHOWWINDOW message in the active document and set focus to the document if it is visible. This
ensures that the page gets focused correctly.

Fixes bug http://code.google.com/p/chromium/issues/detail?id=48459

Fixes bug http://code.google.com/p/chromium/issues/detail?id=25890

Bug=48459, 25890
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=53206

[amit, 2010-07-21 13:10:50]

drive by lgtm!  Does this also fix:  Issue
25890<http://code.google.com/p/chromium/issues/detail?id=25890>
?

On Tue, Jul 20, 2010 at 8:19 PM, <ananta@chromium.org> wrote:

> Reviewers: tommi,
>
> Description:
> Fix incorrect focus cycling issues in ChromeFrame full tab mode. This
> happens
> whenever a tab rendering a chrome frame
> page receives focus by switching to it. Whenever we receive focus in
> ChromeFrame
> we send over an IPC to set the initial
> focus to Chrome. In this IPc we invoke the FocusThroughTabTraversal
> function on
> the TabContents which basically cycles
> through the focus on the page which is not correct for full tab mode.
>
> Fix is to handle the WM_SETFOCUS message in the active document and invoke
> the
> GiveFocusToChrome with false indicating
> that we don't want to invoke the FocusThroughTabTraversal function which
> cycles
> through the view. It is sufficient to
> set focus to the tab.
>
> We also handle the WM_SHOWWINDOW message in the active document and set
> focus to
> the document if it is visible. This
> ensures that the page gets focused correctly.
>
> Fixes bug http://code.google.com/p/chromium/issues/detail?id=48459
>
> Bug=48459
>
>
> Please review this at http://codereview.chromium.org/2825061/show
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     chrome_frame/chrome_active_document.h
>  M     chrome_frame/chrome_active_document.cc
>
>
> Index: chrome_frame/chrome_active_document.cc
> ===================================================================
> --- chrome_frame/chrome_active_document.cc      (revision 53106)
> +++ chrome_frame/chrome_active_document.cc      (working copy)
> @@ -1264,6 +1264,21 @@
>   return 0;
>  }
>
> +LRESULT ChromeActiveDocument::OnShowWindow(UINT message, WPARAM wparam,
> +                                           LPARAM lparam,
> +                                           BOOL& handled) {  // NO_LINT
> +  if (wparam)
> +    SetFocus();
> +  return 0;
> +}
> +
> +LRESULT ChromeActiveDocument::OnSetFocus(UINT message, WPARAM wparam,
> +                                         LPARAM lparam,
> +                                         BOOL& handled) {  // NO_LINT
> +  GiveFocusToChrome(false);
> +  return 0;
> +}
> +
>  namespace {
>  struct ModuleAndVersion {
>   const char* module_name_;
> Index: chrome_frame/chrome_active_document.h
> ===================================================================
> --- chrome_frame/chrome_active_document.h       (revision 53106)
> +++ chrome_frame/chrome_active_document.h       (working copy)
> @@ -236,6 +236,8 @@
>   MESSAGE_HANDLER(WM_FIRE_PRIVACY_CHANGE_NOTIFICATION, OnFirePrivacyChange)
>   COMMAND_ID_HANDLER(IDC_CHROMEFRAME_FORWARD, OnForward)
>   COMMAND_ID_HANDLER(IDC_CHROMEFRAME_BACK, OnBack)
> +  MESSAGE_HANDLER(WM_SHOWWINDOW, OnShowWindow)
> +  MESSAGE_HANDLER(WM_SETFOCUS, OnSetFocus)
>   CHAIN_MSG_MAP(BaseActiveX)
>  END_MSG_MAP()
>
> @@ -340,7 +342,7 @@
>   bool PreProcessContextMenu(HMENU menu);
>   bool HandleContextMenuCommand(UINT cmd, const IPC::ContextMenuParams&
> params);
>
> -  // ChromeFramePlugin overrides.
> + // ChromeFramePlugin overrides.
>   virtual void OnAutomationServerReady();
>
>   // IEnumPrivacyRecords
> @@ -421,6 +423,10 @@
>
>   LRESULT OnFirePrivacyChange(UINT message, WPARAM wparam, LPARAM lparam,
>                               BOOL& handled);
> +  LRESULT OnShowWindow(UINT message, WPARAM wparam, LPARAM lparam,
> +                       BOOL& handled);
> +  LRESULT OnSetFocus(UINT message, WPARAM wparam, LPARAM lparam,
> +                     BOOL& handled);
>
>   // Checks for the presence of known-to-be-buggy BHOs.  If we find any
>   // we do not fire the DocumentComplete event to avoid a crash.
>
>
>

[tommi (sloooow) - chröme, 2010-07-21 14:06:46]

lgtm

[ananta, 2010-07-21 16:47:51]

On 2010/07/21 13:10:50, amit wrote:
> drive by lgtm!  Does this also fix:  Issue
> 25890<http://code.google.com/p/chromium/issues/detail?id=25890>
> ?

Yes

> 
> On Tue, Jul 20, 2010 at 8:19 PM, <mailto:ananta@chromium.org> wrote:
> 
> > Reviewers: tommi,
> >
> > Description:
> > Fix incorrect focus cycling issues in ChromeFrame full tab mode. This
> > happens
> > whenever a tab rendering a chrome frame
> > page receives focus by switching to it. Whenever we receive focus in
> > ChromeFrame
> > we send over an IPC to set the initial
> > focus to Chrome. In this IPc we invoke the FocusThroughTabTraversal
> > function on
> > the TabContents which basically cycles
> > through the focus on the page which is not correct for full tab mode.
> >
> > Fix is to handle the WM_SETFOCUS message in the active document and invoke
> > the
> > GiveFocusToChrome with false indicating
> > that we don't want to invoke the FocusThroughTabTraversal function which
> > cycles
> > through the view. It is sufficient to
> > set focus to the tab.
> >
> > We also handle the WM_SHOWWINDOW message in the active document and set
> > focus to
> > the document if it is visible. This
> > ensures that the page gets focused correctly.
> >
> > Fixes bug http://code.google.com/p/chromium/issues/detail?id=48459
> >
> > Bug=48459
> >
> >
> > Please review this at http://codereview.chromium.org/2825061/show
> >
> > SVN Base: svn://svn.chromium.org/chrome/trunk/src/
> >
> > Affected files:
> >  M     chrome_frame/chrome_active_document.h
> >  M     chrome_frame/chrome_active_document.cc
> >
> >
> > Index: chrome_frame/chrome_active_document.cc
> > ===================================================================
> > --- chrome_frame/chrome_active_document.cc      (revision 53106)
> > +++ chrome_frame/chrome_active_document.cc      (working copy)
> > @@ -1264,6 +1264,21 @@
> >   return 0;
> >  }
> >
> > +LRESULT ChromeActiveDocument::OnShowWindow(UINT message, WPARAM wparam,
> > +                                           LPARAM lparam,
> > +                                           BOOL& handled) {  // NO_LINT
> > +  if (wparam)
> > +    SetFocus();
> > +  return 0;
> > +}
> > +
> > +LRESULT ChromeActiveDocument::OnSetFocus(UINT message, WPARAM wparam,
> > +                                         LPARAM lparam,
> > +                                         BOOL& handled) {  // NO_LINT
> > +  GiveFocusToChrome(false);
> > +  return 0;
> > +}
> > +
> >  namespace {
> >  struct ModuleAndVersion {
> >   const char* module_name_;
> > Index: chrome_frame/chrome_active_document.h
> > ===================================================================
> > --- chrome_frame/chrome_active_document.h       (revision 53106)
> > +++ chrome_frame/chrome_active_document.h       (working copy)
> > @@ -236,6 +236,8 @@
> >   MESSAGE_HANDLER(WM_FIRE_PRIVACY_CHANGE_NOTIFICATION, OnFirePrivacyChange)
> >   COMMAND_ID_HANDLER(IDC_CHROMEFRAME_FORWARD, OnForward)
> >   COMMAND_ID_HANDLER(IDC_CHROMEFRAME_BACK, OnBack)
> > +  MESSAGE_HANDLER(WM_SHOWWINDOW, OnShowWindow)
> > +  MESSAGE_HANDLER(WM_SETFOCUS, OnSetFocus)
> >   CHAIN_MSG_MAP(BaseActiveX)
> >  END_MSG_MAP()
> >
> > @@ -340,7 +342,7 @@
> >   bool PreProcessContextMenu(HMENU menu);
> >   bool HandleContextMenuCommand(UINT cmd, const IPC::ContextMenuParams&
> > params);
> >
> > -  // ChromeFramePlugin overrides.
> > + // ChromeFramePlugin overrides.
> >   virtual void OnAutomationServerReady();
> >
> >   // IEnumPrivacyRecords
> > @@ -421,6 +423,10 @@
> >
> >   LRESULT OnFirePrivacyChange(UINT message, WPARAM wparam, LPARAM lparam,
> >                               BOOL& handled);
> > +  LRESULT OnShowWindow(UINT message, WPARAM wparam, LPARAM lparam,
> > +                       BOOL& handled);
> > +  LRESULT OnSetFocus(UINT message, WPARAM wparam, LPARAM lparam,
> > +                     BOOL& handled);
> >
> >   // Checks for the presence of known-to-be-buggy BHOs.  If we find any
> >   // we do not fire the DocumentComplete event to avoid a crash.
> >
> >
> >
>