Refactored cWebPlugin for C++11 style and proper WebTab clearing.

src/Bindings/ManualBindings.cpp

@@ -2647,22 +2647,16 @@ static int tolua_AllToLua_cWebAdmin_GetURLEncodedString(lua_State * tolua_S)
 
 static int tolua_cWebPlugin_GetTabNames(lua_State * tolua_S)
 {
-	cWebPlugin* self = (cWebPlugin*)  tolua_tousertype(tolua_S, 1, nullptr);
+	// Returns a map of (SafeTitle -> Title) for the plugin's web tabs.
-
+	auto self = reinterpret_cast<cWebPlugin *>(tolua_tousertype(tolua_S, 1, nullptr));
-	const cWebPlugin::TabNameList & TabNames = self->GetTabNames();
+	auto TabNames = self->GetTabNames();
-
 	lua_newtable(tolua_S);
 	int index = 1;
-	cWebPlugin::TabNameList::const_iterator iter = TabNames.begin();
+	for (auto itr = TabNames.cbegin(), end = TabNames.cend(); itr != end; ++itr)
-	while (iter != TabNames.end())
 	{
-		const AString & FancyName = iter->first;
+		tolua_pushstring(tolua_S, itr->second.c_str());  // Because the SafeTitle is supposed to be unique, use it as key
-		const AString & WebName = iter->second;
+		tolua_pushstring(tolua_S, itr->first.c_str());
-		tolua_pushstring(tolua_S, WebName.c_str());  // Because the WebName is supposed to be unique, use it as key
-		tolua_pushstring(tolua_S, FancyName.c_str());
-		//
 		lua_rawset(tolua_S, -3);
-		++iter;
 		++index;
 	}
 	return 1;

src/Bindings/PluginLua.cpp

@@ -200,6 +200,7 @@ bool cPluginLua::Load(void)
 
 void cPluginLua::Unload(void)
 {
+	ClearTabs();
 	super::Unload();
 	Close();
 }
@@ -2000,40 +2001,29 @@ void cPluginLua::AddResettable(cPluginLua::cResettablePtr a_Resettable)
 
 
 
-AString cPluginLua::HandleWebRequest(const HTTPRequest * a_Request)
+AString cPluginLua::HandleWebRequest(const HTTPRequest & a_Request)
 {
-	cCSLock Lock(m_CriticalSection);
+	// Find the tab to use for the request:
-	std::string RetVal = "";
+	auto TabName = GetTabNameForRequest(a_Request);
-
+	AString SafeTabTitle = TabName.second;
-	std::pair< std::string, std::string > TabName = GetTabNameForRequest(a_Request);
+	if (SafeTabTitle.empty())
-	std::string SafeTabName = TabName.second;
+	{
-	if (SafeTabName.empty())
+		return "";
+	}
+	auto Tab = GetTabBySafeTitle(SafeTabTitle);
+	if (Tab == nullptr)
 	{
 		return "";
 	}
 
-	sWebPluginTab * Tab = 0;
+	// Get the page content from the plugin:
-	for (TabList::iterator itr = GetTabs().begin(); itr != GetTabs().end(); ++itr)
+	cCSLock Lock(m_CriticalSection);
+	AString Contents = Printf("WARNING: WebPlugin tab '%s' did not return a string!", Tab->m_Title.c_str());
+	if (!m_LuaState.Call(Tab->m_UserData, &a_Request, cLuaState::Return, Contents))
 	{
-		if ((*itr)->SafeTitle.compare(SafeTabName) == 0)  // This is the one! Rawr
+		return "Lua encountered error while processing the page request";
-		{
-			Tab = *itr;
-			break;
-		}
 	}
-
+	return Contents;
-	if (Tab != nullptr)
-	{
-		AString Contents = Printf("WARNING: WebPlugin tab '%s' did not return a string!", Tab->Title.c_str());
-		if (!m_LuaState.Call(Tab->UserData, a_Request, cLuaState::Return, Contents))
-		{
-			return "Lua encountered error while processing the page request";
-		}
-
-		RetVal += Contents;
-	}
-
-	return RetVal;
 }
 
 
@@ -2048,13 +2038,7 @@ bool cPluginLua::AddWebTab(const AString & a_Title, lua_State * a_LuaState, int
 		LOGERROR("Only allowed to add a tab to a WebPlugin of your own Plugin!");
 		return false;
 	}
-	sWebPluginTab * Tab = new sWebPluginTab();
+	AddNewWebTab(a_Title, a_FunctionReference);
-	Tab->Title = a_Title;
-	Tab->SafeTitle = SafeString(a_Title);
-
-	Tab->UserData = a_FunctionReference;
-
-	GetTabs().push_back(Tab);
 	return true;
 }
 

src/Bindings/PluginLua.h

@@ -176,12 +176,13 @@ public:
 	/** Returns true if the plugin contains the function for the specified hook type, using the old-style registration (#121) */
 	bool CanAddOldStyleHook(int a_HookType);
 	
-	// cWebPlugin override
+	// cWebPlugin overrides
 	virtual const AString GetWebTitle(void) const {return GetName(); }
+	virtual AString HandleWebRequest(const HTTPRequest & a_Request) override;
 
-	// cWebPlugin and WebAdmin stuff
+	/** Adds a new web tab to webadmin.
-	virtual AString HandleWebRequest(const HTTPRequest * a_Request) override;
+	Displaying the tab calls the referenced function. */
-	bool AddWebTab(const AString & a_Title, lua_State * a_LuaState, int a_FunctionReference);  // >> EXPORTED IN MANUALBINDINGS <<
+	bool AddWebTab(const AString & a_Title, lua_State * a_LuaState, int a_FunctionReference);  // Exported in ManualBindings.cpp
 	
 	/** Binds the command to call the function specified by a Lua function reference. Simply adds to CommandMap. */
 	void BindCommand(const AString & a_Command, int a_FnRef);

src/Bindings/WebPlugin.cpp

@@ -24,32 +24,26 @@ cWebPlugin::cWebPlugin()
 
 cWebPlugin::~cWebPlugin()
 {
+	ASSERT(m_Tabs.empty());  // Has ClearTabs() been called?
+
+	// Remove from WebAdmin:
 	cWebAdmin * WebAdmin = cRoot::Get()->GetWebAdmin();
 	if (WebAdmin != nullptr)
 	{
 		WebAdmin->RemovePlugin(this);
 	}
-
-	for (TabList::iterator itr = m_Tabs.begin(); itr != m_Tabs.end(); ++itr)
-	{
-		delete *itr;
-	}
-	m_Tabs.clear();
 }
 
 
 
 
 
-std::list<std::pair<AString, AString> > cWebPlugin::GetTabNames(void)
+cWebPlugin::cTabNames cWebPlugin::GetTabNames(void) const
 {
-	std::list< std::pair< AString, AString > > NameList;
+	std::list< std::pair<AString, AString>> NameList;
-	for (TabList::iterator itr = GetTabs().begin(); itr != GetTabs().end(); ++itr)
+	for (auto itr = m_Tabs.cbegin(), end = m_Tabs.cend(); itr != end; ++itr)
 	{
-		std::pair< AString, AString > StringPair;
+		NameList.push_back(std::make_pair((*itr)->m_Title, (*itr)->m_SafeTitle));
-		StringPair.first = (*itr)->Title;
-		StringPair.second = (*itr)->SafeTitle;
-		NameList.push_back( StringPair);
 	}
 	return NameList;
 }
@@ -58,41 +52,54 @@ std::list<std::pair<AString, AString> > cWebPlugin::GetTabNames(void)
 
 
 
-std::pair< AString, AString > cWebPlugin::GetTabNameForRequest(const HTTPRequest * a_Request)
+cWebPlugin::cTabPtr cWebPlugin::GetTabBySafeTitle(const AString & a_SafeTitle) const
 {
-	std::pair< AString, AString > Names;
+	cCSLock Lock(m_CSTabs);
-	AStringVector Split = StringSplit(a_Request->Path, "/");
+	for (auto itr = m_Tabs.cbegin(), end = m_Tabs.cend(); itr != end; ++itr)
-
-	if (Split.size() > 1)
 	{
-		sWebPluginTab * Tab = nullptr;
+		if ((*itr)->m_SafeTitle == a_SafeTitle)
-		if (Split.size() > 2)  // If we got the tab name, show that page
 		{
-			for (TabList::iterator itr = GetTabs().begin(); itr != GetTabs().end(); ++itr)
+			return *itr;
-			{
-				if ((*itr)->SafeTitle.compare(Split[2]) == 0)  // This is the one!
-				{
-					Tab = *itr;
-					break;
-				}
-			}
-		}
-		else  // Otherwise show the first tab
-		{
-			if (GetTabs().size() > 0)
-			{
-				Tab = *GetTabs().begin();
-			}
-		}
-
-		if (Tab != nullptr)
-		{
-			Names.first = Tab->Title;
-			Names.second = Tab->SafeTitle;
 		}
 	}
+	return nullptr;
+}
 
-	return Names;
+
+
+
+
+std::pair<AString, AString> cWebPlugin::GetTabNameForRequest(const HTTPRequest & a_Request)
+{
+	AStringVector Split = StringSplit(a_Request.Path, "/");
+	if (Split.empty())
+	{
+		return std::make_pair(AString(), AString());
+	}
+
+	cCSLock Lock(m_CSTabs);
+	cTabPtr Tab;
+	if (Split.size() > 2)  // If we got the tab name, show that page
+	{
+		for (auto itr = m_Tabs.cbegin(), end = m_Tabs.cend(); itr != end; ++itr)
+		{
+			if ((*itr)->m_SafeTitle.compare(Split[2]) == 0)  // This is the one!
+			{
+				return std::make_pair((*itr)->m_Title, (*itr)->m_SafeTitle);
+			}
+		}
+		// Tab name not found, display an "empty" page:
+		return std::make_pair(AString(), AString());
+	}
+
+	// Show the first tab:
+	if (!m_Tabs.empty())
+	{
+		return std::make_pair(m_Tabs.front()->m_SafeTitle, m_Tabs.front()->m_SafeTitle);
+	}
+
+	// No tabs at all:
+	return std::make_pair(AString(), AString());
 }
 
 
@@ -101,14 +108,16 @@ std::pair< AString, AString > cWebPlugin::GetTabNameForRequest(const HTTPRequest
 AString cWebPlugin::SafeString(const AString & a_String)
 {
 	AString RetVal;
-	for (unsigned int i = 0; i < a_String.size(); ++i)
+	auto len = a_String.size();
+	RetVal.reserve(len);
+	for (size_t i = 0; i < len; ++i)
 	{
 		char c = a_String[i];
 		if (c == ' ')
 		{
 			c = '_';
 		}
-		RetVal.push_back( c);
+		RetVal.push_back(c);
 	}
 	return RetVal;
 }
@@ -116,3 +125,28 @@ AString cWebPlugin::SafeString(const AString & a_String)
 
 
 
+
+void cWebPlugin::AddNewWebTab(const AString & a_Title, int a_UserData)
+{
+	auto Tab = std::make_shared<cTab>(a_Title, a_UserData);
+	cCSLock Lock(m_CSTabs);
+	m_Tabs.push_back(Tab);
+}
+
+
+
+
+
+void cWebPlugin::ClearTabs(void)
+{
+	// Remove the webadmin tabs:
+	cTabPtrs Tabs;
+	{
+		cCSLock Lock(m_CSTabs);
+		std::swap(Tabs, m_Tabs);
+	}
+}
+
+
+
+

src/Bindings/WebPlugin.h

@@ -12,34 +12,67 @@ class cWebPlugin
 {
 public:
 	// tolua_end
+
+	struct cTab
+	{
+		AString m_Title;
+		AString m_SafeTitle;
+		int m_UserData;
+
+		cTab(const AString & a_Title, int a_UserData):
+			m_Title(a_Title),
+			m_SafeTitle(cWebPlugin::SafeString(a_Title)),
+			m_UserData(a_UserData)
+		{
+		}
+	};
+
+	typedef SharedPtr<cTab> cTabPtr;
+	typedef std::list<cTabPtr> cTabPtrs;
+	typedef std::list<std::pair<AString, AString>> cTabNames;
+
+
 	cWebPlugin();
+
 	virtual ~cWebPlugin();
 
 	// tolua_begin
+
+	/** Returns the title of the plugin, as it should be presented in the webadmin's pages tree. */
 	virtual const AString GetWebTitle(void) const = 0;
 	
-	virtual AString HandleWebRequest(const HTTPRequest * a_Request) = 0;
+	/** Sanitizes the input string, replacing spaces with underscores. */
+	static AString SafeString(const AString & a_String);
 
-	static AString SafeString( const AString & a_String);
 	// tolua_end
 
-	struct sWebPluginTab
+	virtual AString HandleWebRequest(const HTTPRequest & a_Request) = 0;
-	{
-		std::string Title;
-		std::string SafeTitle;
 
-		int UserData;
+	/** Adds a new web tab with the specified contents. */
-	};
+	void AddNewWebTab(const AString & a_Title, int a_UserData);
 
-	typedef std::list< sWebPluginTab* > TabList;
+	/** Removes all the tabs. */
-	TabList & GetTabs() { return m_Tabs; }
+	void ClearTabs(void);
 
-	typedef std::list< std::pair<AString, AString> > TabNameList;
+	/** Returns all the tabs that this plugin has registered. */
-	TabNameList GetTabNames();  // >> EXPORTED IN MANUALBINDINGS <<
+	const cTabPtrs & GetTabs(void) const { return m_Tabs; }
-	std::pair< AString, AString > GetTabNameForRequest(const HTTPRequest* a_Request);
+
+	/** Returns all of the tabs that this plugin has registered. */
+	cTabNames GetTabNames(void) const;  // Exported in ManualBindings.cpp
+
+	/** Returns the tab that has the specified SafeTitle.
+	Returns nullptr if no such tab. */
+	cTabPtr GetTabBySafeTitle(const AString & a_SafeTitle) const;
+
+	std::pair<AString, AString> GetTabNameForRequest(const HTTPRequest & a_Request);
 
 private:
-	TabList m_Tabs;
+	/** All tabs that this plugin has registered.
+	Protected against multithreaded access by m_CSTabs. */
+	cTabPtrs m_Tabs;
+
+	/** Protects m_Tabs against multithreaded access. */
+	mutable cCriticalSection m_CSTabs;
 };  // tolua_export
 
 

src/WebAdmin.cpp

@@ -464,10 +464,10 @@ sWebAdminPage cWebAdmin::GetPage(const HTTPRequest & a_Request)
 		{
 			if ((*itr)->GetWebTitle() == Split[1])
 			{
-				Page.Content = (*itr)->HandleWebRequest(&a_Request);
+				Page.Content = (*itr)->HandleWebRequest(a_Request);
 				cWebPlugin * WebPlugin = *itr;
 				FoundPlugin = WebPlugin->GetWebTitle();
-				AString TabName = WebPlugin->GetTabNameForRequest(&a_Request).first;
+				AString TabName = WebPlugin->GetTabNameForRequest(a_Request).first;
 				Page.PluginName = FoundPlugin;
 				Page.TabName = TabName;
 				break;