Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(113)

Unified Diff: content/browser/find_request_manager.cc

Issue 2249133002: Changes to how FindRequestManager reports find results. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Expanded comment for FinalUpdate(), and renamed to FinalUpdateReceived(). Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/browser/find_request_manager.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/find_request_manager.cc
diff --git a/content/browser/find_request_manager.cc b/content/browser/find_request_manager.cc
index ba97753698a77a8e8f00fe80181909fb083d17a5..db706a283a7ccf9267e7669048611d6fac0939c9 100644
--- a/content/browser/find_request_manager.cc
+++ b/content/browser/find_request_manager.cc
@@ -91,11 +91,13 @@ FindRequestManager::FindRequestManager(WebContentsImpl* web_contents)
: WebContentsObserver(web_contents),
contents_(web_contents),
current_session_id_(kInvalidId),
+ pending_find_next_reply_(nullptr),
pending_active_match_ordinal_(false),
number_of_matches_(0),
active_frame_(nullptr),
relative_active_match_ordinal_(0),
- active_match_ordinal_(0) {}
+ active_match_ordinal_(0),
+ last_reported_id_(kInvalidId) {}
FindRequestManager::~FindRequestManager() {}
@@ -190,16 +192,21 @@ void FindRequestManager::OnFindReply(RenderFrameHost* rfh,
// This is the final update for this frame for the current find operation.
- pending_replies_.erase(rfh);
- if (request_id == current_session_id_ && !pending_replies_.empty()) {
+ pending_initial_replies_.erase(rfh);
+ if (request_id == current_session_id_ && !pending_initial_replies_.empty()) {
NotifyFindReply(request_id, false /* final_update */);
return;
}
- DCHECK(request_id == current_session_id_ ||
- current_request_.options.findNext);
// This is the final update for the current find operation.
- FinalUpdate(request_id, rfh);
+
+ if (request_id == current_request_.id && request_id != current_session_id_) {
+ DCHECK(current_request_.options.findNext);
+ DCHECK_EQ(pending_find_next_reply_, rfh);
+ pending_find_next_reply_ = nullptr;
+ }
+
+ FinalUpdateReceived(request_id, rfh);
}
void FindRequestManager::RemoveFrame(RenderFrameHost* rfh) {
@@ -218,6 +225,7 @@ void FindRequestManager::RemoveFrame(RenderFrameHost* rfh) {
if (active_frame_ == rfh) {
active_frame_ = nullptr;
relative_active_match_ordinal_ = 0;
+ selection_rect_ = gfx::Rect();
}
UpdateActiveMatchOrdinal();
@@ -239,17 +247,28 @@ void FindRequestManager::RemoveFrame(RenderFrameHost* rfh) {
RemoveFindMatchRectsPendingReply(rfh);
#endif
- if (pending_replies_.count(rfh)) {
+ // If no pending find replies are expected for the removed frame, then just
+ // report the updated results.
+ if (!pending_initial_replies_.count(rfh) && pending_find_next_reply_ != rfh) {
+ bool final_update =
+ pending_initial_replies_.empty() && !pending_find_next_reply_;
+ NotifyFindReply(current_session_id_, final_update);
+ return;
+ }
+
+ if (pending_initial_replies_.count(rfh)) {
// A reply should not be expected from the removed frame.
- pending_replies_.erase(rfh);
- if (pending_replies_.empty()) {
- FinalUpdate(current_request_.id, rfh);
- return;
+ pending_initial_replies_.erase(rfh);
+ if (pending_initial_replies_.empty()) {
+ FinalUpdateReceived(current_session_id_, rfh);
}
}
- NotifyFindReply(current_session_id_,
- pending_replies_.empty() /* final_update */);
+ if (pending_find_next_reply_ == rfh) {
+ // A reply should not be expected from the removed frame.
+ pending_find_next_reply_ = nullptr;
+ FinalUpdateReceived(current_request_.id, rfh);
+ }
}
#if defined(OS_ANDROID)
@@ -264,7 +283,7 @@ void FindRequestManager::ActivateNearestFindResult(float x, float y) {
for (FrameTreeNode* node : contents_->GetFrameTree()->Nodes()) {
RenderFrameHost* rfh = node->current_frame_host();
- if (!CheckFrame(rfh))
+ if (!CheckFrame(rfh) || !rfh->IsRenderFrameLive())
continue;
activate_.pending_replies.insert(rfh);
@@ -299,7 +318,7 @@ void FindRequestManager::RequestFindMatchRects(int current_version) {
for (FrameTreeNode* node : contents_->GetFrameTree()->Nodes()) {
RenderFrameHost* rfh = node->current_frame_host();
- if (!CheckFrame(rfh))
+ if (!CheckFrame(rfh) || !rfh->IsRenderFrameLive())
continue;
match_rects_.pending_replies.insert(rfh);
@@ -343,7 +362,8 @@ void FindRequestManager::FrameDeleted(RenderFrameHost* rfh) {
void FindRequestManager::Reset(const FindRequest& initial_request) {
current_session_id_ = initial_request.id;
current_request_ = initial_request;
- pending_replies_.clear();
+ pending_initial_replies_.clear();
+ pending_find_next_reply_ = nullptr;
pending_active_match_ordinal_ = true;
matches_per_frame_.clear();
number_of_matches_ = 0;
@@ -351,6 +371,7 @@ void FindRequestManager::Reset(const FindRequest& initial_request) {
relative_active_match_ordinal_ = 0;
active_match_ordinal_ = 0;
selection_rect_ = gfx::Rect();
+ last_reported_id_ = kInvalidId;
#if defined(OS_ANDROID)
activate_ = ActivateNearestFindResultState();
match_rects_.pending_replies.clear();
@@ -400,18 +421,31 @@ void FindRequestManager::AdvanceQueue(int request_id) {
void FindRequestManager::SendFindIPC(const FindRequest& request,
RenderFrameHost* rfh) {
- pending_replies_.insert(rfh);
+ DCHECK(CheckFrame(rfh));
+ DCHECK(rfh->IsRenderFrameLive());
+
+ if (request.options.findNext)
+ pending_find_next_reply_ = rfh;
+ else
+ pending_initial_replies_.insert(rfh);
+
rfh->Send(new FrameMsg_Find(rfh->GetRoutingID(), request.id,
request.search_text, request.options));
}
-void FindRequestManager::NotifyFindReply(int request_id,
- bool final_update) const {
+void FindRequestManager::NotifyFindReply(int request_id, bool final_update) {
if (request_id == kInvalidId) {
NOTREACHED();
return;
}
+ // Ensure that replies are not reported with IDs lower than the ID of the
+ // latest request we have results from.
+ if (request_id < last_reported_id_)
+ request_id = last_reported_id_;
+ else
+ last_reported_id_ = request_id;
+
contents_->NotifyFindReply(request_id, number_of_matches_, selection_rect_,
active_match_ordinal_, final_update);
}
@@ -437,7 +471,7 @@ RenderFrameHost* FindRequestManager::Traverse(RenderFrameHost* from_rfh,
continue;
RenderFrameHost* current_rfh = node->current_frame_host();
if (!matches_only || matches_per_frame_.find(current_rfh)->second ||
- pending_replies_.count(current_rfh)) {
+ pending_initial_replies_.count(current_rfh)) {
// Note that if there is still a pending reply expected for this frame,
// then it may have unaccounted matches and will not be skipped via
// |matches_only|.
@@ -466,7 +500,7 @@ void FindRequestManager::AddFrame(RenderFrameHost* rfh) {
}
bool FindRequestManager::CheckFrame(RenderFrameHost* rfh) const {
- return rfh && rfh->IsRenderFrameLive() && matches_per_frame_.count(rfh);
+ return rfh && matches_per_frame_.count(rfh);
}
void FindRequestManager::UpdateActiveMatchOrdinal() {
@@ -490,11 +524,16 @@ void FindRequestManager::UpdateActiveMatchOrdinal() {
active_match_ordinal_ += relative_active_match_ordinal_;
}
-void FindRequestManager::FinalUpdate(int request_id, RenderFrameHost* rfh) {
+void FindRequestManager::FinalUpdateReceived(int request_id,
+ RenderFrameHost* rfh) {
if (!number_of_matches_ ||
- !pending_active_match_ordinal_ ||
+ (active_match_ordinal_ && !pending_active_match_ordinal_) ||
request_id != current_request_.id) {
- NotifyFindReply(request_id, true /* final_update */);
+ // All the find results for |request_id| are in and ready to report. Note
+ // that |final_update| will be set to false if there are still pending
+ // replies expected from the initial find request.
+ NotifyFindReply(request_id,
+ pending_initial_replies_.empty() /* final_update */);
AdvanceQueue(request_id);
return;
}
« no previous file with comments | « content/browser/find_request_manager.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698