Skip to content

Commit

Permalink
* Fix reference counting bug in ComPtr.
Browse files Browse the repository at this point in the history
* Move ITfDisplayAttributeProvider to its own class.
  • Loading branch information
PCMan committed Sep 13, 2013
1 parent e14cefe commit 5df8845
Show file tree
Hide file tree
Showing 12 changed files with 233 additions and 102 deletions.
6 changes: 3 additions & 3 deletions ChewingTextService/ChewingTextService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ TextService::TextService(ImeModule* module):
Ime::LangBarButton* button = new Ime::LangBarButton(this, g_settingsButtonGuid);
button->setTooltip(IDS_SETTINGS);
button->setIcon(IDI_CONFIG);
HMENU menu = ::LoadMenuW(this->module()->hInstance(), LPCTSTR(IDR_MENU));
HMENU menu = ::LoadMenuW(this->imeModule()->hInstance(), LPCTSTR(IDR_MENU));
HMENU popup = ::GetSubMenu(menu, 0);
button->setMenu(popup);
addButton(button);
Expand Down Expand Up @@ -368,7 +368,7 @@ bool TextService::onCommand(UINT id) {
case ID_ABOUT: // show about dialog
if(!isImmersive()) { // only do this in desktop app mode
Ime::Dialog dlg;
dlg.showModal(this->module()->hInstance(), IDD_ABOUT);
dlg.showModal(this->imeModule()->hInstance(), IDD_ABOUT);
}
break;
case ID_WEBSITE: // visit chewing website
Expand Down Expand Up @@ -411,7 +411,7 @@ bool TextService::onConfigure(HWND hwndParent) {
Ime::PropertyDialog dlg;
TypingPage* typingPage = new TypingPage();
dlg.addPage(typingPage);
dlg.showModal(this->module()->hInstance(), (LPCTSTR)IDS_CONFIG_TITLE, 0, hwndParent);
dlg.showModal(this->imeModule()->hInstance(), (LPCTSTR)IDS_CONFIG_TITLE, 0, hwndParent);
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion ChewingTextService/ChewingTextService.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class TextService: public Ime::TextService {
}

Config& config() {
return static_cast<ImeModule*>(module())->config();
return static_cast<ImeModule*>(imeModule())->config();
}

bool hasCandidates() {
Expand Down
2 changes: 2 additions & 0 deletions libIME/ComPtr.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ class ComPtr {
}

ComPtr(T* p): p_(p) {
if(p_)
p_->AddRef();
}

ComPtr(const ComPtr& other): p_(other.p_) {
Expand Down
21 changes: 13 additions & 8 deletions libIME/DisplayAttributeInfoEnum.cpp
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
#include "DisplayAttributeInfoEnum.h"
#include "TextService.h"
#include "DisplayAttributeProvider.h"
#include "ImeModule.h"
#include <assert.h>

using namespace Ime;

DisplayAttributeInfoEnum::DisplayAttributeInfoEnum(TextService* service):
textService_(service),
DisplayAttributeInfoEnum::DisplayAttributeInfoEnum(DisplayAttributeProvider* provider):
provider_(provider),
refCount_(1) {
iterator_ = service->displayAttrInfos_.begin();
std::list<DisplayAttributeInfo*>& displayAttrInfos = provider_->imeModule_->displayAttrInfos();
iterator_ = displayAttrInfos.begin();
}

DisplayAttributeInfoEnum::DisplayAttributeInfoEnum(const DisplayAttributeInfoEnum& other):
textService_(other.textService_),
provider_(other.provider_),
iterator_(other.iterator_) {
}

Expand Down Expand Up @@ -56,8 +58,9 @@ STDMETHODIMP DisplayAttributeInfoEnum::Clone(IEnumTfDisplayAttributeInfo **ppEnu

STDMETHODIMP DisplayAttributeInfoEnum::Next(ULONG ulCount, ITfDisplayAttributeInfo **rgInfo, ULONG *pcFetched) {
ULONG i = 0;
std::list<DisplayAttributeInfo*>& displayAttrInfos = provider_->imeModule_->displayAttrInfos();
for(; i < ulCount; ++i) {
if(iterator_ != textService_->displayAttrInfos_.end()) {
if(iterator_ != displayAttrInfos.end()) {
DisplayAttributeInfo* info = *iterator_;
info->AddRef();
rgInfo[i] = info;
Expand All @@ -72,12 +75,14 @@ STDMETHODIMP DisplayAttributeInfoEnum::Next(ULONG ulCount, ITfDisplayAttributeIn
}

STDMETHODIMP DisplayAttributeInfoEnum::Reset() {
iterator_ = textService_->displayAttrInfos_.begin();
std::list<DisplayAttributeInfo*>& displayAttrInfos = provider_->imeModule_->displayAttrInfos();
iterator_ = displayAttrInfos.begin();
return S_OK;
}

STDMETHODIMP DisplayAttributeInfoEnum::Skip(ULONG ulCount) {
if(iterator_ != textService_->displayAttrInfos_.end())
std::list<DisplayAttributeInfo*>& displayAttrInfos = provider_->imeModule_->displayAttrInfos();
if(iterator_ != displayAttrInfos.end())
++iterator_;
return S_OK;
}
6 changes: 3 additions & 3 deletions libIME/DisplayAttributeInfoEnum.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@

namespace Ime {

class TextService;
class DisplayAttributeProvider;

class DisplayAttributeInfoEnum : public IEnumTfDisplayAttributeInfo {
public:
DisplayAttributeInfoEnum(TextService* service);
DisplayAttributeInfoEnum(DisplayAttributeProvider* provider);
DisplayAttributeInfoEnum(const DisplayAttributeInfoEnum& other);
virtual ~DisplayAttributeInfoEnum(void);

Expand All @@ -30,7 +30,7 @@ class DisplayAttributeInfoEnum : public IEnumTfDisplayAttributeInfo {
private:
int refCount_;
std::list<DisplayAttributeInfo*>::iterator iterator_;
ComPtr<TextService> textService_;
ComPtr<DisplayAttributeProvider> provider_;
};

}
Expand Down
69 changes: 69 additions & 0 deletions libIME/DisplayAttributeProvider.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
#include "DisplayAttributeProvider.h"
#include "DisplayAttributeInfo.h"
#include "DisplayAttributeInfoEnum.h"
#include "ImeModule.h"
#include <assert.h>

using namespace Ime;
using namespace std;

DisplayAttributeProvider::DisplayAttributeProvider(ImeModule* module):
imeModule_(module),
refCount_(1) {
}

DisplayAttributeProvider::~DisplayAttributeProvider(void) {
}


// COM stuff

// IUnknown
STDMETHODIMP DisplayAttributeProvider::QueryInterface(REFIID riid, void **ppvObj) {
if (ppvObj == NULL)
return E_INVALIDARG;
if(IsEqualIID(riid, IID_IUnknown) || IsEqualIID(riid, IID_ITfDisplayAttributeProvider))
*ppvObj = (ITfDisplayAttributeProvider*)this;
else
*ppvObj = NULL;

if(*ppvObj) {
AddRef();
return S_OK;
}
return E_NOINTERFACE;
}

// IUnknown implementation
STDMETHODIMP_(ULONG) DisplayAttributeProvider::AddRef(void) {
return ++refCount_;
}

STDMETHODIMP_(ULONG) DisplayAttributeProvider::Release(void) {
assert(refCount_ > 0);
--refCount_;
if(0 == refCount_)
delete this;
return refCount_;
}


// ITfDisplayAttributeProvider
STDMETHODIMP DisplayAttributeProvider::EnumDisplayAttributeInfo(IEnumTfDisplayAttributeInfo **ppEnum) {
*ppEnum = (IEnumTfDisplayAttributeInfo*)new DisplayAttributeInfoEnum(this);
return S_OK;
}

STDMETHODIMP DisplayAttributeProvider::GetDisplayAttributeInfo(REFGUID guidInfo, ITfDisplayAttributeInfo **ppInfo) {
list<DisplayAttributeInfo*>& displayAttrInfos = imeModule_->displayAttrInfos();
list<DisplayAttributeInfo*>::iterator it;
for(it = displayAttrInfos.begin(); it != displayAttrInfos.end(); ++it) {
DisplayAttributeInfo* info = *it;
if(::IsEqualGUID(info->guid(), guidInfo)) {
*ppInfo = info;
info->AddRef();
return S_OK;
}
}
return E_INVALIDARG;
}
41 changes: 41 additions & 0 deletions libIME/DisplayAttributeProvider.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#ifndef IME_DISPLAY_ATTRIBUTE_PROVIDER_H
#define IME_DISPLAY_ATTRIBUTE_PROVIDER_H

#include <msctf.h>
#include <list>
#include "ComPtr.h"

namespace Ime {

class ImeModule;
class DisplayAttributeInfo;

class DisplayAttributeProvider : public ITfDisplayAttributeProvider {
public:

friend class DisplayAttributeInfoEnum;

DisplayAttributeProvider(ImeModule* module);
virtual ~DisplayAttributeProvider(void);

// COM stuff

// IUnknown
STDMETHODIMP QueryInterface(REFIID riid, void **ppvObj);
STDMETHODIMP_(ULONG) AddRef(void);
STDMETHODIMP_(ULONG) Release(void);

// ITfDisplayAttributeProvider
STDMETHODIMP EnumDisplayAttributeInfo(IEnumTfDisplayAttributeInfo **ppEnum);
STDMETHODIMP GetDisplayAttributeInfo(REFGUID guidInfo, ITfDisplayAttributeInfo **ppInfo);

private:
int refCount_;
ComPtr<ImeModule> imeModule_;
};

}

#endif


76 changes: 67 additions & 9 deletions libIME/ImeModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <assert.h>
#include "Window.h"
#include "TextService.h"
#include "DisplayAttributeProvider.h"

using namespace Ime;
using namespace std;
Expand All @@ -21,16 +22,46 @@ static const GUID GUID_TFCAT_TIPCAP_SYSTRAYSUPPORT =
{ 0x25504FB4, 0x7BAB, 0x4BC1, { 0x9C, 0x69, 0xCF, 0x81, 0x89, 0x0F, 0x0E, 0xF5 } };
#endif

// display attribute GUIDs

// {2A2574D6-2841-4F27-82BD-D7B0F23855F8}
static const GUID g_inputDisplayAttributeGuid =
{ 0x2a2574d6, 0x2841, 0x4f27, { 0x82, 0xbd, 0xd7, 0xb0, 0xf2, 0x38, 0x55, 0xf8 } };

// {E1270AA5-A6B1-4112-9AC7-F5E476C3BD63}
static const GUID g_convertedDisplayAttributeGuid =
{ 0xe1270aa5, 0xa6b1, 0x4112, { 0x9a, 0xc7, 0xf5, 0xe4, 0x76, 0xc3, 0xbd, 0x63 } };


ImeModule::ImeModule(HMODULE module, const CLSID& textServiceClsid):
hInstance_(HINSTANCE(module)),
textServiceClsid_(textServiceClsid),
refCount_(1) {

Window::registerClass(hInstance_);

// regiser default display attributes
inputAttrib_ = new DisplayAttributeInfo(g_inputDisplayAttributeGuid);
inputAttrib_->setTextColor(COLOR_WINDOWTEXT);
inputAttrib_->setLineStyle(TF_LS_DOT);
inputAttrib_->setLineColor(COLOR_WINDOWTEXT);
displayAttrInfos_.push_back(inputAttrib_);
convertedAttrib_ = new DisplayAttributeInfo(g_convertedDisplayAttributeGuid);
displayAttrInfos_.push_back(convertedAttrib_);

registerDisplayAttributeInfos();
}

ImeModule::~ImeModule(void) {

// display attributes
if(!displayAttrInfos_.empty()) {
list<DisplayAttributeInfo*>::iterator it;
for(it = displayAttrInfos_.begin(); it != displayAttrInfos_.end(); ++it) {
DisplayAttributeInfo* info = *it;
info->Release();
}
}
}

// Dll entry points implementations
Expand Down Expand Up @@ -178,6 +209,24 @@ HRESULT ImeModule::unregisterServer(const GUID& profileGuid) {
return S_OK;
}


// display attributes stuff
bool ImeModule::registerDisplayAttributeInfos() {

// register display attributes
ComPtr<ITfCategoryMgr> categoryMgr;
if(::CoCreateInstance(CLSID_TF_CategoryMgr, NULL, CLSCTX_INPROC_SERVER, IID_ITfCategoryMgr, (void**)&categoryMgr) == S_OK) {
TfGuidAtom atom;
categoryMgr->RegisterGUID(g_inputDisplayAttributeGuid, &atom);
inputAttrib_->setAtom(atom);
categoryMgr->RegisterGUID(g_convertedDisplayAttributeGuid, &atom);
convertedAttrib_->setAtom(atom);
return true;
}
return false;
}


// COM related stuff

// IUnknown
Expand Down Expand Up @@ -218,15 +267,24 @@ STDMETHODIMP_(ULONG) ImeModule::Release(void) {

// IClassFactory
STDMETHODIMP ImeModule::CreateInstance(IUnknown *pUnkOuter, REFIID riid, void **ppvObj) {
// FIXME: do we need to check riid here?
TextService* service = createTextService();
// FIXME: we should split DisplayAttributeProvider into another class
// Otherwise, everytime a new TextService object is created just for enumerating display attributes.
// This is really a waste and may cause potential side effects.
if(service) {
service->QueryInterface(riid, ppvObj);
service->Release();
return S_OK;
if(::IsEqualIID(riid, IID_ITfDisplayAttributeProvider)) {
DisplayAttributeProvider* provider = new DisplayAttributeProvider(this);
if(provider) {
provider->QueryInterface(riid, ppvObj);
provider->Release();
return S_OK;
}
}
else {
TextService* service = createTextService();
// FIXME: we should split DisplayAttributeProvider into another class
// Otherwise, everytime a new TextService object is created just for enumerating display attributes.
// This is really a waste and may cause potential side effects.
if(service) {
service->QueryInterface(riid, ppvObj);
service->Release();
return S_OK;
}
}
return S_FALSE;
}
Expand Down
23 changes: 23 additions & 0 deletions libIME/ImeModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@

#include <Unknwn.h>
#include <Windows.h>
#include <list>

namespace Ime {

class TextService;
class DisplayAttributeInfo;

class ImeModule: public IClassFactory {
public:
Expand All @@ -28,8 +30,24 @@ class ImeModule: public IClassFactory {
HRESULT registerServer(wchar_t* name, const GUID& profileGuid, LANGID languageId);
HRESULT unregisterServer(const GUID& profileGuid);

// should be override by IME implementors
virtual TextService* createTextService() = 0;

// display attributes for composition string
std::list<DisplayAttributeInfo*>& displayAttrInfos() {
return displayAttrInfos_;
}

bool registerDisplayAttributeInfos();

DisplayAttributeInfo* inputAttrib() {
return inputAttrib_;
}

DisplayAttributeInfo* convertedAttrib() {
return convertedAttrib_;
}

// COM-related stuff

// IUnknown
Expand All @@ -47,6 +65,11 @@ class ImeModule: public IClassFactory {
HINSTANCE hInstance_;
CLSID textServiceClsid_;
wchar_t* tooltip_;

// display attributes
std::list<DisplayAttributeInfo*> displayAttrInfos_; // display attribute info
DisplayAttributeInfo* inputAttrib_;
DisplayAttributeInfo* convertedAttrib_;
};

}
Expand Down
Loading

0 comments on commit 5df8845

Please sign in to comment.