Revert "Attempt to fix bug 9158 using updateBuffer."

This reverts commit fe24616060.

src/Buffer.cpp

@@ -289,6 +289,8 @@ public:
 	mutable BiblioInfo bibinfo_;
 	/// whether the bibinfo cache is valid
 	mutable bool bibinfo_cache_valid_;
+	/// whether the bibfile cache is valid
+	mutable bool bibfile_cache_valid_;
 	/// Cache of timestamps of .bib files
 	map<FileName, time_t> bibfile_status_;
 	/// Indicates whether the bibinfo has changed since the last time
@@ -341,8 +343,10 @@ public:
 		if (!cloned_buffer_ && parent_buffer && pb)
 			LYXERR0("Warning: a buffer should not have two parents!");
 		parent_buffer = pb;
-		if (!cloned_buffer_ && parent_buffer)
+		if (!cloned_buffer_ && parent_buffer) {
+			parent_buffer->invalidateBibfileCache();
 			parent_buffer->invalidateBibinfoCache();
+		}
 	}
 
 	/// If non zero, this buffer is a clone of existing buffer \p cloned_buffer_
@@ -428,7 +432,7 @@ Buffer::Impl::Impl(Buffer * owner, FileName const & file, bool readonly_,
 	  file_fully_loaded(false), file_format(LYX_FORMAT), need_format_backup(false),
 	  ignore_parent(false),  toc_backend(owner), macro_lock(false),
 	  checksum_(0), wa_(0),  gui_(0), undo_(*owner), bibinfo_cache_valid_(false),
-	  cite_labels_valid_(false), preview_error_(false),
+	  bibfile_cache_valid_(false), cite_labels_valid_(false), preview_error_(false),
 	  inset(0), preview_loader_(0), cloned_buffer_(cloned_buffer),
 	  clone_list_(0), doing_export(false),
 	  tracked_changes_present_(0), externally_modified_(false), parent_buffer(0),
@@ -448,6 +452,7 @@ Buffer::Impl::Impl(Buffer * owner, FileName const & file, bool readonly_,
 	bibfiles_cache_ = cloned_buffer_->d->bibfiles_cache_;
 	bibinfo_ = cloned_buffer_->d->bibinfo_;
 	bibinfo_cache_valid_ = cloned_buffer_->d->bibinfo_cache_valid_;
+	bibfile_cache_valid_ = cloned_buffer_->d->bibfile_cache_valid_;
 	bibfile_status_ = cloned_buffer_->d->bibfile_status_;
 	cite_labels_valid_ = cloned_buffer_->d->cite_labels_valid_;
 	unnamed = cloned_buffer_->d->unnamed;
@@ -1899,7 +1904,7 @@ void Buffer::writeLaTeXSource(otexstream & os,
 		// Biblatex bibliographies are loaded here
 		if (params().useBiblatex()) {
 			vector<docstring> const bibfiles =
-				prepareBibFilePaths(runparams, getBibfiles(), true);
+				prepareBibFilePaths(runparams, getBibfilesCache(), true);
 			for (docstring const & file: bibfiles)
 				os << "\\addbibresource{" << file << "}\n";
 		}
@@ -2317,11 +2322,47 @@ void Buffer::getLabelList(vector<docstring> & list) const
 }
 
 
+void Buffer::updateBibfilesCache(UpdateScope scope) const
+{
+	// FIXME This is probably unnecssary, given where we call this.
+	// If this is a child document, use the parent's cache instead.
+	if (parent() && scope != UpdateChildOnly) {
+		masterBuffer()->updateBibfilesCache();
+		return;
+	}
+
+	d->bibfiles_cache_.clear();
+	for (InsetIterator it = inset_iterator_begin(inset()); it; ++it) {
+		if (it->lyxCode() == BIBTEX_CODE) {
+			InsetBibtex const & inset = static_cast<InsetBibtex const &>(*it);
+			support::FileNamePairList const bibfiles = inset.getBibFiles();
+			d->bibfiles_cache_.insert(d->bibfiles_cache_.end(),
+				bibfiles.begin(),
+				bibfiles.end());
+		} else if (it->lyxCode() == INCLUDE_CODE) {
+			InsetInclude & inset = static_cast<InsetInclude &>(*it);
+			Buffer const * const incbuf = inset.getChildBuffer();
+			if (!incbuf)
+				continue;
+			support::FileNamePairList const & bibfiles =
+					incbuf->getBibfilesCache(UpdateChildOnly);
+			if (!bibfiles.empty()) {
+				d->bibfiles_cache_.insert(d->bibfiles_cache_.end(),
+					bibfiles.begin(),
+					bibfiles.end());
+			}
+		}
+	}
+	d->bibfile_cache_valid_ = true;
+	d->bibinfo_cache_valid_ = false;
+	d->cite_labels_valid_ = false;
+}
+
+
 void Buffer::invalidateBibinfoCache() const
 {
 	d->bibinfo_cache_valid_ = false;
 	d->cite_labels_valid_ = false;
-	removeBiblioTempFiles();
 	// also invalidate the cache for the parent buffer
 	Buffer const * const pbuf = d->parent();
 	if (pbuf)
@@ -2329,13 +2370,29 @@ void Buffer::invalidateBibinfoCache() const
 }
 
 
-FileNamePairList const & Buffer::getBibfiles(UpdateScope scope) const
+void Buffer::invalidateBibfileCache() const
+{
+	d->bibfile_cache_valid_ = false;
+	d->bibinfo_cache_valid_ = false;
+	d->cite_labels_valid_ = false;
+	// also invalidate the cache for the parent buffer
+	Buffer const * const pbuf = d->parent();
+	if (pbuf)
+		pbuf->invalidateBibfileCache();
+}
+
+
+support::FileNamePairList const & Buffer::getBibfilesCache(UpdateScope scope) const
 {
 	// FIXME This is probably unnecessary, given where we call this.
-	// If this is a child document, use the master instead.
+	// If this is a child document, use the master's cache instead.
 	Buffer const * const pbuf = masterBuffer();
 	if (pbuf != this && scope != UpdateChildOnly)
-		return pbuf->getBibfiles();
+		return pbuf->getBibfilesCache();
+
+	if (!d->bibfile_cache_valid_)
+		this->updateBibfilesCache(scope);
+
 	return d->bibfiles_cache_;
 }
 
@@ -2355,20 +2412,6 @@ BiblioInfo const & Buffer::bibInfo() const
 }
 
 
-void Buffer::registerBibfiles(FileNamePairList const & bf) const {
-	Buffer const * const tmp = masterBuffer();
-	if (tmp != this)
-		return tmp->registerBibfiles(bf);
-
-	for (auto const & p : bf) {
-		FileNamePairList::const_iterator tmp =
-			find(d->bibfiles_cache_.begin(), d->bibfiles_cache_.end(), p);
-		if (tmp == d->bibfiles_cache_.end())
-			d->bibfiles_cache_.push_back(p);
-	}
-}
-
-
 void Buffer::checkIfBibInfoCacheIsValid() const
 {
 	// use the master's cache
@@ -2378,13 +2421,8 @@ void Buffer::checkIfBibInfoCacheIsValid() const
 		return;
 	}
 
-	// if we already know the cache is invalid, no need to check
-	// the timestamps
-	if (!d->bibinfo_cache_valid_)
-		return;
-
 	// compare the cached timestamps with the actual ones.
-	FileNamePairList const & bibfiles_cache = getBibfiles();
+	FileNamePairList const & bibfiles_cache = getBibfilesCache();
 	FileNamePairList::const_iterator ei = bibfiles_cache.begin();
 	FileNamePairList::const_iterator en = bibfiles_cache.end();
 	for (; ei != en; ++ ei) {
@@ -4723,16 +4761,10 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
 	Buffer const * const master = masterBuffer();
 	DocumentClass const & textclass = master->params().documentClass();
 
-	FileNamePairList old_bibfiles;
 	// do this only if we are the top-level Buffer
 	if (master == this) {
 		textclass.counters().reset(from_ascii("bibitem"));
 		reloadBibInfoCache();
-		// we will re-read this cache as we go through, but we need
-		// to know whether it's changed to know whether we need to
-		// update the bibinfo cache.
-		old_bibfiles = d->bibfiles_cache_;
-		d->bibfiles_cache_.clear();
 	}
 
 	// keep the buffers to be children in this set. If the call from the
@@ -4778,45 +4810,14 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
 	ParIterator parit = cbuf.par_iterator_begin();
 	updateBuffer(parit, utype);
 
-	// If this document has siblings, then update the TocBackend later. The
-	// reason is to ensure that later siblings are up to date when e.g. the
-	// broken or not status of references is computed. The update is called
-	// in InsetInclude::addToToc.
 	if (master != this)
+		// If this document has siblings, then update the TocBackend later. The
+		// reason is to ensure that later siblings are up to date when e.g. the
+		// broken or not status of references is computed. The update is called
+		// in InsetInclude::addToToc.
 		return;
 
-	// if the bibfiles changed, the cache of bibinfo is invalid
+	d->bibinfo_cache_valid_ = true;
-	FileNamePairList new_bibfiles = d->bibfiles_cache_;
-	// this is a trick to determine whether the two vectors have
-	// the same elements.
-	sort(new_bibfiles.begin(), new_bibfiles.end());
-	sort(old_bibfiles.begin(), old_bibfiles.end());
-	if (old_bibfiles != new_bibfiles) {
-		LYXERR(Debug::FILES, "Reloading bibinfo cache.");
-		invalidateBibinfoCache();
-		reloadBibInfoCache();
-		// We relied upon the bibinfo cache when recalculating labels. But that
-		// cache was invalid, although we didn't find that out until now. So we
-		// have to do it all again.
-		// That said, the only thing we really need to do is update the citation
-		// labels. Nothing else will have changed. So we could create a new 
-		// UpdateType that would signal that fact, if we needed to do so.
-		parit = cbuf.par_iterator_begin();
-		// we will be re-doing the counters and references and such.
-		textclass.counters().reset();
-		clearReferenceCache();
-		// we should not need to do this again?
-		// updateMacros();
-		setChangesPresent(false);
-		updateBuffer(parit, utype);
-		// this will already have been done by reloadBibInfoCache();
-		// d->bibinfo_cache_valid_ = true;
-	}
-	else {
-		LYXERR(Debug::FILES, "Bibfiles unchanged.");
-		// this is also set to true on the other path, by reloadBibInfoCache.
-		d->bibinfo_cache_valid_ = true;
-	}
 	d->cite_labels_valid_ = true;
 	/// FIXME: Perf
 	cbuf.tocBackend().update(true, utype);

src/Buffer.h

@@ -493,13 +493,25 @@ public:
 	*/
 	void validate(LaTeXFeatures &) const;
 
-	/// Bibliography information is cached in the Buffer, so we do not
+	/// Reference information is cached in the Buffer, so we do not
 	/// have to check or read things over and over.
-	/// The cache exists only in the master buffer. When it is updated,
+	///
+	/// There are two caches.
+	///
+	/// One is a cache of the BibTeX files from which reference info is
+	/// being gathered. This cache is PER BUFFER, and the cache for the
+	/// master essentially includes the cache for its children. This gets
+	/// invalidated when an InsetBibtex is created, deleted, or modified.
+	///
+	/// The other is a cache of the reference information itself. This
+	/// exists only in the master buffer, and when it needs to be updated,
 	/// the children add their information to the master's cache.
+
 	/// Calling this method invalidates the cache and so requires a
 	/// re-read.
 	void invalidateBibinfoCache() const;
+	/// This invalidates the cache of files we need to check.
+	void invalidateBibfileCache() const;
 	/// Updates the cached bibliography information, checking first to see
 	/// whether the cache is valid. If so, we do nothing. If not, then we
 	/// reload all the BibTeX info.
@@ -767,8 +779,6 @@ public:
 	void setChangesPresent(bool) const;
 	bool areChangesPresent() const;
 	void updateChangesPresent() const;
-	///
-	void registerBibfiles(support::FileNamePairList const & bf) const;
 
 private:
 	friend class MarkAsExporting;
@@ -785,10 +795,13 @@ private:
 	/// last time we loaded the cache. Note that this does NOT update the
 	/// cached information.
 	void checkIfBibInfoCacheIsValid() const;
+	/// Update the list of all bibfiles in use (including bibfiles
+	/// of loaded child documents).
+	void updateBibfilesCache(UpdateScope scope = UpdateMaster) const;
 	/// Return the list with all bibfiles in use (including bibfiles
 	/// of loaded child documents).
 	support::FileNamePairList const &
-		getBibfiles(UpdateScope scope = UpdateMaster) const;
+		getBibfilesCache(UpdateScope scope = UpdateMaster) const;
 	///
 	void collectChildren(ListOfBuffers & children, bool grand_children) const;
 

src/BufferView.cpp

@@ -1663,8 +1663,10 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
 		InsetBibtex * inset = getInsetByCode<InsetBibtex>(tmpcur,
 						BIBTEX_CODE);
 		if (inset) {
-			if (inset->addDatabase(cmd.argument()))
+			if (inset->addDatabase(cmd.argument())) {
+				buffer_.invalidateBibfileCache();
 				dr.forceBufferUpdate();
+			}
 		}
 		break;
 	}
@@ -1675,8 +1677,10 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
 		InsetBibtex * inset = getInsetByCode<InsetBibtex>(tmpcur,
 						BIBTEX_CODE);
 		if (inset) {
-			if (inset->delDatabase(cmd.argument()))
+			if (inset->delDatabase(cmd.argument())) {
+				buffer_.invalidateBibfileCache();
 				dr.forceBufferUpdate();
+			}
 		}
 		break;
 	}

src/insets/InsetBibtex.cpp

@@ -59,7 +59,22 @@ namespace os = support::os;
 
 InsetBibtex::InsetBibtex(Buffer * buf, InsetCommandParams const & p)
 	: InsetCommand(buf, p)
-{}
+{
+	buffer().invalidateBibfileCache();
+	buffer().removeBiblioTempFiles();
+}
+
+
+InsetBibtex::~InsetBibtex()
+{
+	if (isBufferLoaded()) {
+		/* We do not use buffer() because Coverity believes that this
+		 * may throw an exception. Actually this code path is not
+		 * taken when buffer_ == 0 */
+		buffer_-> invalidateBibfileCache();
+		buffer_->removeBiblioTempFiles();
+	}
+}
 
 
 ParamInfo const & InsetBibtex::findInfo(string const & /* cmdName */)
@@ -101,6 +116,8 @@ void InsetBibtex::doDispatch(Cursor & cur, FuncRequest & cmd)
 
 		cur.recordUndo();
 		setParams(p);
+		buffer().invalidateBibfileCache();
+		buffer().removeBiblioTempFiles();
 		cur.forceBufferUpdate();
 		break;
 	}
@@ -360,15 +377,15 @@ void InsetBibtex::latex(otexstream & os, OutputParams const & runparams) const
 }
 
 
-FileNamePairList InsetBibtex::getBibFiles() const
+support::FileNamePairList InsetBibtex::getBibFiles() const
 {
 	FileName path(buffer().filePath());
-	PathChanger p(path);
+	support::PathChanger p(path);
 
 	// We need to store both the real FileName and the way it is entered
 	// (with full path, rel path or as a single file name).
 	// The latter is needed for biblatex's central bibfile macro.
-	FileNamePairList vec;
+	support::FileNamePairList vec;
 
 	vector<docstring> bibfilelist = getVectorFromString(getParam("bibfiles"));
 	vector<docstring>::const_iterator it = bibfilelist.begin();
@@ -383,6 +400,7 @@ FileNamePairList InsetBibtex::getBibFiles() const
 	}
 
 	return vec;
+
 }
 
 namespace {
@@ -655,9 +673,9 @@ void InsetBibtex::parseBibTeXFiles(FileNameList & checkedFiles) const
 
 	BiblioInfo keylist;
 
-	FileNamePairList const files = getBibFiles();
+	support::FileNamePairList const files = getBibFiles();
-	FileNamePairList::const_iterator it = files.begin();
+	support::FileNamePairList::const_iterator it = files.begin();
-	FileNamePairList::const_iterator en = files.end();
+	support::FileNamePairList::const_iterator en = files.end();
 	for (; it != en; ++ it) {
 		FileName const bibfile = it->second;
 		if (find(checkedFiles.begin(), checkedFiles.end(), bibfile) != checkedFiles.end())
@@ -893,11 +911,6 @@ void InsetBibtex::validate(LaTeXFeatures & features) const
 }
 
 
-void InsetBibtex::updateBuffer(ParIterator const &, UpdateType) {
-	buffer().registerBibfiles(getBibFiles());
-}
-
-
 int InsetBibtex::plaintext(odocstringstream & os,
        OutputParams const & op, size_t max_length) const
 {

src/insets/InsetBibtex.h

@@ -29,6 +29,8 @@ class InsetBibtex : public InsetCommand {
 public:
 	///
 	InsetBibtex(Buffer *, InsetCommandParams const &);
+	///
+	~InsetBibtex();
 
 	///
 	support::FileNamePairList getBibFiles() const;
@@ -55,8 +57,6 @@ public:
 	int plaintext(odocstringstream & ods, OutputParams const & op,
 	              size_t max_length = INT_MAX) const;
 	///
-	void updateBuffer(ParIterator const &, UpdateType);
-	///
 	void collectBibKeys(InsetIterator const &, support::FileNameList &) const;
 	///
 	void validate(LaTeXFeatures &) const;

src/insets/InsetInclude.cpp

@@ -197,6 +197,11 @@ InsetInclude::InsetInclude(InsetInclude const & other)
 
 InsetInclude::~InsetInclude()
 {
+	if (isBufferLoaded())
+		/* We do not use buffer() because Coverity believes that this
+		 * may throw an exception. Actually this code path is not
+		 * taken when buffer_ == 0 */
+		buffer_->invalidateBibfileCache();
 	delete label_;
 }
 
@@ -349,6 +354,8 @@ void InsetInclude::setParams(InsetCommandParams const & p)
 
 	if (type(params()) == INPUT)
 		add_preview(*preview_, *this, buffer());
+
+	buffer().invalidateBibfileCache();
 }
 
 

src/insets/InsetInclude.h

@@ -59,6 +59,14 @@ public:
 	 */
 	void updateBibfilesCache();
 
+	/** Return the cache with all bibfiles in use of the child buffer
+	 *  (including bibfiles of grandchild documents).
+	 *  Return an empty vector if the child doc is not loaded.
+	 *  \param buffer the Buffer containing this inset.
+	 */
+	support::FileNameList const &
+		getBibfilesCache() const;
+
 	///
 	void updateCommand();
 	///