EXT-8459 FIXED preventing crashes: 1) ensure that pointer to inventory item is still valid when landmark is loaded from notecard and 2) adding a check for region capability

There are two reasons of the crash reported in the bug:
 * absence of the "CopyInventoryFromNotecard" capability in region (which leads to crash while logging of a LL_ERRS)
 * referencing to an invalid pointer to LLInventoryItem in callback.


The first issue is fixed by preventing sending of the "CopyInventoryFromNotecard" message if it is not supported
 (in the "copy_inventory_from_notecard()")

The second issue caused by such reason:
 * Notecard stores LLPointer to each embedded inventory item
 * When Landmark is clicked it should be opened in Places Panel and inventory item should copied into agent inventory
 * If it is unknown to agent it is requested and pointer (but not LLPointer!) to inventory item was bound to an appropriate callback
 * Then when landmark is loaded that inventory item is copied to inventory.
 * If notecard was closed before callback was trigged all instances to embedded inventory items were destroyed.
This leads to crash when trigged callback tries to reference to bound pointer to inventory item (for landmarks)

Fix is to pass LLPointer instead of pointer to inventory item into callback to ensure item is valid when it is needed.
Details:
 * updated LLEmbeddedItems::getEmbeddedItem() to return LLPointer to inventory item (and renamed to getEmbeddedItemPtr)
 * updated LLViewerTextEditor::openEmbeddedItem() to get LLPointer to inventory item
 * updated LLViewerTextEditor::openEmbeddedLandmark() to get LLPointer to inventory item

Patch also contains some more places where pointer is replaced with LLPointer to be consistent.

NOTE: there are several LLViewerTextEditor::openEmbeddedXXX() methods which still get pointer to inventory item.
 It is safe for now because they use it synchronously. I have added a note at their declaration.

Reviewed by Vadim Savchuk at https://codereview.productengine.com/secondlife/r/784/

--HG--
branch : product-engine

3 files changed, 52 insertions, 23 deletions

diff --git a/indra/newview/llviewerinventory.cpp b/indra/newview/llviewerinventory.cpp
index 2d57c16889..5d90af0cfe 100644
--- a/indra/newview/llviewerinventory.cpp
+++ b/indra/newview/llviewerinventory.cpp
@@ -1172,6 +1172,14 @@ void move_inventory_item(
 
 void copy_inventory_from_notecard(const LLUUID& object_id, const LLUUID& notecard_inv_id, const LLInventoryItem *src, U32 callback_id)
 {
+	if (NULL == src)
+	{
+		LL_WARNS("copy_inventory_from_notecard") << "Null pointer to item was passed for object_id "
+												 << object_id << " and notecard_inv_id "
+												 << notecard_inv_id << LL_ENDL;
+		return;
+	}
+
 	LLViewerRegion* viewer_region = NULL;
     LLViewerObject* vo = NULL;
 	if (object_id.notNull() && (vo = gObjectList.findObject(object_id)) != NULL)
@@ -1194,6 +1202,16 @@ void copy_inventory_from_notecard(const LLUUID& object_id, const LLUUID& notecar
         return;
     }
 
+	// check capability to prevent a crash while LL_ERRS in LLCapabilityListener::capListener. See EXT-8459.
+	std::string url = viewer_region->getCapability("CopyInventoryFromNotecard");
+	if (url.empty())
+	{
+        LL_WARNS("copy_inventory_from_notecard") << "There is no 'CopyInventoryFromNotecard' capability"
+												 << " for region: " << viewer_region->getName()
+                                                 << LL_ENDL;
+		return;
+	}
+
     LLSD request, body;
     body["notecard-id"] = notecard_inv_id;
     body["object-id"] = object_id;
diff --git a/indra/newview/llviewertexteditor.cpp b/indra/newview/llviewertexteditor.cpp
index 59efae4cb2..8bd43bb30c 100644
--- a/indra/newview/llviewertexteditor.cpp
+++ b/indra/newview/llviewertexteditor.cpp
@@ -90,7 +90,7 @@ public:
 	}
 	static void processForeignLandmark(LLLandmark* landmark,
 			const LLUUID& object_id, const LLUUID& notecard_inventory_id,
-			LLInventoryItem* item)
+			LLPointer<LLInventoryItem> item_ptr)
 	{
 		LLVector3d global_pos;
 		landmark->getGlobalPos(global_pos);
@@ -103,8 +103,16 @@ public:
 		}
 		else
 		{
-			LLPointer<LLEmbeddedLandmarkCopied> cb = new LLEmbeddedLandmarkCopied();
-			copy_inventory_from_notecard(object_id, notecard_inventory_id, item, gInventoryCallbacks.registerCB(cb));
+			if (item_ptr.isNull())
+			{
+				// check to prevent a crash. See EXT-8459.
+				llwarns << "Passed handle contains a dead inventory item. Most likely notecard has been closed and embedded item was destroyed." << llendl;
+			}
+			else
+			{
+				LLPointer<LLEmbeddedLandmarkCopied> cb = new LLEmbeddedLandmarkCopied();
+				copy_inventory_from_notecard(object_id, notecard_inventory_id, item_ptr.get(), gInventoryCallbacks.registerCB(cb));
+			}
 		}
 	}
 };
@@ -300,14 +308,14 @@ public:
 
 	void	markSaved();
 	
-	static LLInventoryItem* getEmbeddedItem(llwchar ext_char); // returns item from static list
+	static LLPointer<LLInventoryItem> getEmbeddedItemPtr(llwchar ext_char); // returns pointer to item from static list
 	static BOOL getEmbeddedItemSaved(llwchar ext_char); // returns whether item from static list is saved
 
 private:
 
 	struct embedded_info_t
 	{
-		LLPointer<LLInventoryItem> mItem;
+		LLPointer<LLInventoryItem> mItemPtr;
 		BOOL mSaved;
 	};
 	typedef std::map<llwchar, embedded_info_t > item_map_t;
@@ -378,7 +386,7 @@ BOOL LLEmbeddedItems::insertEmbeddedItem( LLInventoryItem* item, llwchar* ext_ch
 		++wc_emb;
 	}
 
-	sEntries[wc_emb].mItem = item;
+	sEntries[wc_emb].mItemPtr = item;
 	sEntries[wc_emb].mSaved = is_new ? FALSE : TRUE;
 	*ext_char = wc_emb;
 	mEmbeddedUsedChars.insert(wc_emb);
@@ -400,14 +408,14 @@ BOOL LLEmbeddedItems::removeEmbeddedItem( llwchar ext_char )
 }
 	
 // static
-LLInventoryItem* LLEmbeddedItems::getEmbeddedItem(llwchar ext_char)
+LLPointer<LLInventoryItem> LLEmbeddedItems::getEmbeddedItemPtr(llwchar ext_char)
 {
 	if( ext_char >= LLTextEditor::FIRST_EMBEDDED_CHAR && ext_char <= LLTextEditor::LAST_EMBEDDED_CHAR )
 	{
 		item_map_t::iterator iter = sEntries.find(ext_char);
 		if (iter != sEntries.end())
 		{
-			return iter->second.mItem;
+			return iter->second.mItemPtr;
 		}
 	}
 	return NULL;
@@ -505,7 +513,7 @@ BOOL LLEmbeddedItems::hasEmbeddedItem(llwchar ext_char)
 
 LLUIImagePtr LLEmbeddedItems::getItemImage(llwchar ext_char) const
 {
-	LLInventoryItem* item = getEmbeddedItem(ext_char);
+	LLInventoryItem* item = getEmbeddedItemPtr(ext_char);
 	if (item)
 	{
 		const char* img_name = "";
@@ -567,7 +575,7 @@ void LLEmbeddedItems::getEmbeddedItemList( std::vector<LLPointer<LLInventoryItem
 	for (std::set<llwchar>::iterator iter = mEmbeddedUsedChars.begin(); iter != mEmbeddedUsedChars.end(); ++iter)
 	{
 		llwchar wc = *iter;
-		LLPointer<LLInventoryItem> item = getEmbeddedItem(wc);
+		LLPointer<LLInventoryItem> item = getEmbeddedItemPtr(wc);
 		if (item)
 		{
 			items.push_back(item);
@@ -698,7 +706,7 @@ BOOL LLViewerTextEditor::handleMouseDown(S32 x, S32 y, MASK mask)
 			{
 				wc = getWText()[mCursorPos];
 			}
-			LLInventoryItem* item_at_pos = LLEmbeddedItems::getEmbeddedItem(wc);
+			LLPointer<LLInventoryItem> item_at_pos = LLEmbeddedItems::getEmbeddedItemPtr(wc);
 			if (item_at_pos)
 			{
 				mDragItem = item_at_pos;
@@ -1019,7 +1027,7 @@ llwchar LLViewerTextEditor::pasteEmbeddedItem(llwchar ext_char)
 	{
 		return ext_char; // already exists in my list
 	}
-	LLInventoryItem* item = LLEmbeddedItems::getEmbeddedItem(ext_char);
+	LLInventoryItem* item = LLEmbeddedItems::getEmbeddedItemPtr(ext_char);
 	if (item)
 	{
 		// Add item to my list and return new llwchar associated with it
@@ -1053,7 +1061,7 @@ void LLViewerTextEditor::findEmbeddedItemSegments(S32 start, S32 end)
 			&& embedded_char <= LAST_EMBEDDED_CHAR 
 			&& mEmbeddedItemList->hasEmbeddedItem(embedded_char) )
 		{
-			LLInventoryItem* itemp = mEmbeddedItemList->getEmbeddedItem(embedded_char);
+			LLInventoryItem* itemp = mEmbeddedItemList->getEmbeddedItemPtr(embedded_char);
 			LLUIImagePtr image = mEmbeddedItemList->getItemImage(embedded_char);
 			insertSegment(new LLEmbeddedItemSegment(idx, image, itemp, *this));
 		}
@@ -1065,7 +1073,7 @@ BOOL LLViewerTextEditor::openEmbeddedItemAtPos(S32 pos)
 	if( pos < getLength())
 	{
 		llwchar wc = getWText()[pos];
-		LLInventoryItem* item = LLEmbeddedItems::getEmbeddedItem( wc );
+		LLPointer<LLInventoryItem> item = LLEmbeddedItems::getEmbeddedItemPtr( wc );
 		if( item )
 		{
 			BOOL saved = LLEmbeddedItems::getEmbeddedItemSaved( wc );
@@ -1083,7 +1091,7 @@ BOOL LLViewerTextEditor::openEmbeddedItemAtPos(S32 pos)
 }
 
 
-BOOL LLViewerTextEditor::openEmbeddedItem(LLInventoryItem* item, llwchar wc)
+BOOL LLViewerTextEditor::openEmbeddedItem(LLPointer<LLInventoryItem> item, llwchar wc)
 {
 
 	switch( item->getType() )
@@ -1151,17 +1159,17 @@ void LLViewerTextEditor::openEmbeddedSound( LLInventoryItem* item, llwchar wc )
 }
 
 
-void LLViewerTextEditor::openEmbeddedLandmark( LLInventoryItem* item, llwchar wc )
+void LLViewerTextEditor::openEmbeddedLandmark( LLPointer<LLInventoryItem> item_ptr, llwchar wc )
 {
-	if (!item)
+	if (item_ptr.isNull())
 		return;
 
-	LLLandmark* landmark = gLandmarkList.getAsset(item->getAssetUUID(),
-			boost::bind(&LLEmbeddedLandmarkCopied::processForeignLandmark, _1, mObjectID, mNotecardInventoryID, item));
+	LLLandmark* landmark = gLandmarkList.getAsset(item_ptr->getAssetUUID(),
+			boost::bind(&LLEmbeddedLandmarkCopied::processForeignLandmark, _1, mObjectID, mNotecardInventoryID, item_ptr));
 	if (landmark)
 	{
 		LLEmbeddedLandmarkCopied::processForeignLandmark(landmark, mObjectID,
-				mNotecardInventoryID, item);
+				mNotecardInventoryID, item_ptr);
 	}
 }
 
@@ -1220,7 +1228,7 @@ bool LLViewerTextEditor::onCopyToInvDialog(const LLSD& notification, const LLSD&
 	{
 		LLUUID item_id = notification["payload"]["item_id"].asUUID();
 		llwchar wc = llwchar(notification["payload"]["item_wc"].asInteger());
-		LLInventoryItem* itemp = LLEmbeddedItems::getEmbeddedItem(wc);
+		LLInventoryItem* itemp = LLEmbeddedItems::getEmbeddedItemPtr(wc);
 		if (itemp)
 			copyInventory(itemp);
 	}
diff --git a/indra/newview/llviewertexteditor.h b/indra/newview/llviewertexteditor.h
index ba0c40cb2e..74b6d70640 100644
--- a/indra/newview/llviewertexteditor.h
+++ b/indra/newview/llviewertexteditor.h
@@ -104,13 +104,16 @@ private:
 	virtual llwchar	pasteEmbeddedItem(llwchar ext_char);
 
 	BOOL			openEmbeddedItemAtPos( S32 pos );
-	BOOL			openEmbeddedItem(LLInventoryItem* item, llwchar wc);
+	BOOL			openEmbeddedItem(LLPointer<LLInventoryItem> item, llwchar wc);
 
 	S32				insertEmbeddedItem(S32 pos, LLInventoryItem* item);
 
+	// *NOTE: most of openEmbeddedXXX methods except openEmbeddedLandmark take pointer to LLInventoryItem.
+	// Be sure they don't bind it to callback function to avoid situation when it gets invalid when
+	// callback is trigged after text editor is closed. See EXT-8459.
 	void			openEmbeddedTexture( LLInventoryItem* item, llwchar wc );
 	void			openEmbeddedSound( LLInventoryItem* item, llwchar wc );
-	void			openEmbeddedLandmark( LLInventoryItem* item, llwchar wc );
+	void			openEmbeddedLandmark( LLPointer<LLInventoryItem> item_ptr, llwchar wc );
 	void			openEmbeddedNotecard( LLInventoryItem* item, llwchar wc);
 	void			openEmbeddedCallingcard( LLInventoryItem* item, llwchar wc);
 	void			showCopyToInvDialog( LLInventoryItem* item, llwchar wc );