Merge 53786 - Our InPlaceMenu implementation is only partially being used....

Merge 53786 - Our InPlaceMenu implementation is only partially being used.
We had a leftover call to InPlaceMenuDestroy without creating a menu in the first place.
This caused us to delete IE's menu and fail the first check inside ieframe!CDocObjectHost::OnInitMenuPopup.
It took a while to figure this out and even longer to figure out why our menu was NULL in this case and not mshtml's.

I don't think that there are any bad side effects with this change but I ask the reviewer to check.

The visible change is that the Print menu item will become enabled and visible whereas Page Setup will be disabled and Print Preview will (correctly atm) not be visible for GCF documents.

BUG=24034, 47074
TEST=See description above and bug report.

Review URL: http://codereview.chromium.org/3034033

TBR=tommi@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=54063

[tommi (sloooow) - chröme, 2010-07-29 07:05:03]

lgtm. thanks

On Wed, Jul 28, 2010 at 7:03 PM, <ananta@chromium.org> wrote:

> Reviewers: tommi,
>
> Description:
> Merge 53786 - Our InPlaceMenu implementation is only partially being used.
> We had a leftover call to InPlaceMenuDestroy without creating a menu in the
> first place.
> This caused us to delete IE's menu and fail the first check inside
> ieframe!CDocObjectHost::OnInitMenuPopup.
> It took a while to figure this out and even longer to figure out why our
> menu
> was NULL in this case and not mshtml's.
>
> I don't think that there are any bad side effects with this change but I
> ask the
> reviewer to check.
>
> The visible change is that the Print menu item will become enabled and
> visible
> whereas Page Setup will be disabled and Print Preview will (correctly atm)
> not
> be visible for GCF documents.
>
> BUG=24034, 47074
> TEST=See description above and bug report.
>
> Review URL: http://codereview.chromium.org/3034033
>
> TBR=tommi@chromium.org
>
> Please review this at http://codereview.chromium.org/2836074/show
>
> SVN Base: svn://svn.chromium.org/chrome/branches/472/src/
>
> Affected files:
>  M     chrome_frame/ole_document_impl.h
>
>
> Index: chrome_frame/ole_document_impl.h
> ===================================================================
> --- chrome_frame/ole_document_impl.h    (revision 54062)
> +++ chrome_frame/ole_document_impl.h    (working copy)
> @@ -209,7 +209,11 @@
>         hr = t->ActiveXDocActivate(OLEIVERB_UIACTIVATE);
>       }
>     } else {
> -      t->InPlaceMenuDestroy();
> +      // Menu integration is still not complete, so do not destroy
> +      // IE's menus.  If we call InPlaceMenuDestroy here, menu items such
> +      // as Print etc will be disabled and we will not get calls to
> QueryStatus
> +      // for those commands.
> +      // t->InPlaceMenuDestroy();
>       // t->DestroyToolbar();
>       hr = t->UIDeactivate();
>     }
>
>
>