Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: src/plugin/PluginClass.cpp

Issue 6453924974297088: Issue 1619 - Wrong return value of CPluginClass::Invoke (Closed)
Left Patch Set: Created Nov. 26, 2014, 8:51 p.m.
Right Patch Set: clean CPluginClass::Invoke Created Nov. 27, 2014, 11:40 a.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « src/plugin/PluginClass.h ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 #include "PluginStdAfx.h" 1 #include "PluginStdAfx.h"
2 2
3 #include "PluginClass.h" 3 #include "PluginClass.h"
4 #include "PluginSettings.h" 4 #include "PluginSettings.h"
5 #include "PluginSystem.h" 5 #include "PluginSystem.h"
6 #include "PluginFilter.h" 6 #include "PluginFilter.h"
7 #include "PluginMimeFilterClient.h" 7 #include "PluginMimeFilterClient.h"
8 #include "PluginClient.h" 8 #include "PluginClient.h"
9 #include "PluginClientFactory.h" 9 #include "PluginClientFactory.h"
10 #include "PluginMutex.h" 10 #include "PluginMutex.h"
(...skipping 570 matching lines...) Expand 10 before | Expand all | Expand 10 after
581 #endif 581 #endif
582 582
583 UpdateStatusBar(); 583 UpdateStatusBar();
584 } 584 }
585 else 585 else
586 { 586 {
587 DEBUG_NAVI(L"Navi::Begin navigation url:" + url) 587 DEBUG_NAVI(L"Navi::Begin navigation url:" + url)
588 m_tab->CacheFrame(url); 588 m_tab->CacheFrame(url);
589 } 589 }
590 } 590 }
591 STDMETHODIMP CPluginClass::OnTabChanged(DISPPARAMS* pDispParams, WORD wFlags) 591 void CPluginClass::OnTabChanged(DISPPARAMS* pDispParams, WORD wFlags)
592 { 592 {
593 DEBUG_GENERAL("Tab changed"); 593 DEBUG_GENERAL("Tab changed");
594 bool newtabshown = pDispParams->rgvarg[1].intVal==3; 594 bool newtabshown = pDispParams->rgvarg[1].intVal==3;
595 if (newtabshown) 595 if (newtabshown)
596 { 596 {
597 std::map<DWORD,CPluginClass*>::const_iterator it = s_threadInstances.find(Ge tCurrentThreadId()); 597 std::map<DWORD,CPluginClass*>::const_iterator it = s_threadInstances.find(Ge tCurrentThreadId());
598 if (it == s_threadInstances.end()) 598 if (it == s_threadInstances.end())
599 { 599 {
600 s_threadInstances[::GetCurrentThreadId()] = this; 600 s_threadInstances[::GetCurrentThreadId()] = this;
601
602
603 if (!m_isInitializedOk) 601 if (!m_isInitializedOk)
604 { 602 {
605 m_isInitializedOk = true; 603 m_isInitializedOk = true;
606 if (!InitObject(true)) 604 if (!InitObject(true))
607 { 605 {
608 // Unadvice(); 606 // Unadvice();
609 } 607 }
610 UpdateStatusBar(); 608 UpdateStatusBar();
611 } 609 }
612 } 610 }
613 } 611 }
614 notificationMessage.Hide(); 612 notificationMessage.Hide();
615 DEBUG_GENERAL("Tab change end"); 613 DEBUG_GENERAL("Tab change end");
616 return VARIANT_TRUE;
617 } 614 }
618 615
619 // This gets called whenever there's a browser event 616 // This gets called whenever there's a browser event
620 STDMETHODIMP CPluginClass::Invoke(DISPID dispidMember, REFIID riid, LCID lcid, W ORD wFlags, DISPPARAMS* pDispParams, VARIANT* pvarResult, EXCEPINFO* pExcepInfo, UINT* puArgErr) 617 STDMETHODIMP CPluginClass::Invoke(DISPID dispidMember, REFIID riid, LCID lcid, W ORD wFlags, DISPPARAMS* pDispParams, VARIANT* pvarResult, EXCEPINFO* pExcepInfo, UINT* puArgErr)
621 { 618 {
622 WCHAR tmp[256]; 619 WCHAR tmp[256];
623 wsprintf(tmp, L"Invoke: %d\n", dispidMember); 620 wsprintf(tmp, L"Invoke: %d\n", dispidMember);
624 DEBUG_GENERAL(tmp); 621 DEBUG_GENERAL(tmp);
625 switch (dispidMember) 622 switch (dispidMember)
626 { 623 {
627
628 case DISPID_WINDOWSTATECHANGED: 624 case DISPID_WINDOWSTATECHANGED:
629 return OnTabChanged(pDispParams, wFlags); 625 OnTabChanged(pDispParams, wFlags);
630 break; 626 return S_OK;
631 case DISPID_HTMLDOCUMENTEVENTS2_ONBEFOREUPDATE:
632 return VARIANT_TRUE;
633 break;
634
635 case DISPID_HTMLDOCUMENTEVENTS2_ONCLICK:
636 return VARIANT_TRUE;
637 break;
638
639 case DISPID_EVMETH_ONLOAD:
640 DEBUG_NAVI("Navi::OnLoad")
641 return VARIANT_TRUE;
642 break;
643
644 case DISPID_EVMETH_ONCHANGE:
645 return VARIANT_TRUE;
646
647 case DISPID_EVMETH_ONMOUSEDOWN:
648 return VARIANT_TRUE;
649
650 case DISPID_EVMETH_ONMOUSEENTER:
651 return VARIANT_TRUE;
652
653 case DISPID_IHTMLIMGELEMENT_START:
654 return VARIANT_TRUE;
655
656 case STDDISPID_XOBJ_ERRORUPDATE:
657 return VARIANT_TRUE;
658
659 case STDDISPID_XOBJ_ONPROPERTYCHANGE:
660 return VARIANT_TRUE;
661
662 case DISPID_READYSTATECHANGE:
663 DEBUG_NAVI("Navi::ReadyStateChange")
664 return VARIANT_TRUE;
665
666 case DISPID_BEFORENAVIGATE:
667 DEBUG_NAVI("Navi::BeforeNavigate")
668 return VARIANT_TRUE;
669 case DISPID_COMMANDSTATECHANGE: 627 case DISPID_COMMANDSTATECHANGE:
670 if (m_hPaneWnd == NULL) 628 if (m_hPaneWnd == NULL)
671 { 629 {
672 CreateStatusBarPane(); 630 CreateStatusBarPane();
673 } 631 }
674 else 632 else
675 { 633 {
676 if (CPluginClient::GetInstance()->GetIEVersion() > 6) 634 if (CPluginClient::GetInstance()->GetIEVersion() > 6)
677 { 635 {
678 RECT rect; 636 RECT rect;
679 BOOL rectRes = GetClientRect(m_hStatusBarWnd, &rect); 637 BOOL rectRes = GetClientRect(m_hStatusBarWnd, &rect);
680 if (rectRes == TRUE) 638 if (rectRes == TRUE)
681 { 639 {
682 MoveWindow(m_hPaneWnd, rect.right - 200, 0, m_nPaneWidth, rect.bottom - rect.top, TRUE); 640 MoveWindow(m_hPaneWnd, rect.right - 200, 0, m_nPaneWidth, rect.bottom - rect.top, TRUE);
683 } 641 }
684 } 642 }
685 } 643 }
686 break; 644 return S_OK;
687 case DISPID_STATUSTEXTCHANGE:
688 break;
689
690 case DISPID_BEFORENAVIGATE2: 645 case DISPID_BEFORENAVIGATE2:
691 BeforeNavigate2(pDispParams); 646 BeforeNavigate2(pDispParams);
692 break; 647 return S_OK;
693
694 case DISPID_DOWNLOADBEGIN:
695 {
696 DEBUG_NAVI("Navi::Download Begin")
697 }
698 break;
699
700 case DISPID_DOWNLOADCOMPLETE: 648 case DISPID_DOWNLOADCOMPLETE:
701 { 649 {
702 DEBUG_NAVI("Navi::Download Complete") 650 DEBUG_NAVI("Navi::Download Complete")
703 651 ATL::CComQIPtr<IWebBrowser2> browser = GetBrowser();
704 CComQIPtr<IWebBrowser2> browser = GetBrowser();
705 if (browser) 652 if (browser)
706 { 653 {
707 m_tab->OnDownloadComplete(browser); 654 m_tab->OnDownloadComplete(browser);
708 } 655 }
709 } 656 }
710 break; 657 return S_OK;
711
712 case DISPID_DOCUMENTCOMPLETE: 658 case DISPID_DOCUMENTCOMPLETE:
713 { 659 {
714 DEBUG_NAVI("Navi::Document Complete") 660 DEBUG_NAVI("Navi::Document Complete")
715 661 ATL::CComPtr<IWebBrowser2> browser = GetBrowser();
Eric 2014/12/30 14:37:50 We could declare this as IWebBrowser2* with one sm
sergei 2015/01/13 13:24:21 I find it as a bad practice, `GetBrowser()` return
716 CComQIPtr<IWebBrowser2> browser = GetBrowser();
717
718 if (browser && pDispParams->cArgs >= 2 && pDispParams->rgvarg[1].vt == VT_ DISPATCH) 662 if (browser && pDispParams->cArgs >= 2 && pDispParams->rgvarg[1].vt == VT_ DISPATCH)
719 { 663 {
720 CComQIPtr<IWebBrowser2> pBrowser = pDispParams->rgvarg[1].pdispVal; 664 ATL::CComQIPtr<IWebBrowser2> pBrowser = pDispParams->rgvarg[1].pdispVal;
Eric 2014/12/30 14:37:50 We can't avoid a QI call to initialize pBrowser2 b
721 if (pBrowser) 665 if (pBrowser)
722 { 666 {
723 CString url; 667 CString url;
724 CComBSTR bstrUrl; 668 ATL::CComBSTR bstrUrl;
725 if (SUCCEEDED(pBrowser->get_LocationURL(&bstrUrl)) && ::SysStringLen(b strUrl) > 0) 669 if (SUCCEEDED(pBrowser->get_LocationURL(&bstrUrl)) && ::SysStringLen(b strUrl) > 0)
726 { 670 {
727 url = bstrUrl; 671 url = bstrUrl;
728
729 CPluginClient::UnescapeUrl(url); 672 CPluginClient::UnescapeUrl(url);
730
731 m_tab->OnDocumentComplete(browser, url, browser.IsEqualObject(pBrows er)); 673 m_tab->OnDocumentComplete(browser, url, browser.IsEqualObject(pBrows er));
Eric 2014/12/30 14:37:50 Change browser.IsEqualObject(pBrowser) to
732 } 674 }
733 } 675 }
734 } 676 }
735 } 677 }
736 break; 678 return S_OK;
737 679
738 case DISPID_ONQUIT: 680 case DISPID_ONQUIT:
739 case DISPID_QUIT: 681 case DISPID_QUIT:
740 { 682 {
741 Unadvice(); 683 Unadvice();
742 } 684 }
743 break; 685 return S_OK;
744
745 default: 686 default:
746 { 687 {
747 CString did; 688 ATL::CString did;
748 did.Format(L"DispId:%u", dispidMember); 689 did.Format(L"DispId:%u", dispidMember);
749
750 DEBUG_NAVI(L"Navi::Default " + did) 690 DEBUG_NAVI(L"Navi::Default " + did)
751 } 691 }
752 692 }
753 // do nothing 693 return DISP_E_MEMBERNOTFOUND;
Eric 2014/12/30 14:37:50 Does this work on all supported versions of IE? T
754 break;
755 }
756
757 return S_OK;
758 } 694 }
759 695
760 bool CPluginClass::InitObject(bool bBHO) 696 bool CPluginClass::InitObject(bool bBHO)
761 { 697 {
762 DEBUG_GENERAL("InitObject"); 698 DEBUG_GENERAL("InitObject");
763 CPluginSettings* settings = CPluginSettings::GetInstance(); 699 CPluginSettings* settings = CPluginSettings::GetInstance();
764 700
765 if (!settings->GetPluginEnabled()) 701 if (!settings->GetPluginEnabled())
766 { 702 {
767 s_mimeFilter->Unregister(); 703 s_mimeFilter->Unregister();
(...skipping 1096 matching lines...) Expand 10 before | Expand all | Expand 10 after
1864 } 1800 }
1865 } 1801 }
1866 } 1802 }
1867 1803
1868 hTabWnd = ::GetWindow(hTabWnd, GW_HWNDNEXT); 1804 hTabWnd = ::GetWindow(hTabWnd, GW_HWNDNEXT);
1869 } 1805 }
1870 1806
1871 return hTabWnd; 1807 return hTabWnd;
1872 1808
1873 } 1809 }
LEFTRIGHT

Powered by Google App Engine
This is Rietveld