Skip to content

Commit

Permalink
Update entry cache in DidFinishNavigation()
Browse files Browse the repository at this point in the history
Previously we were persisting the SpeedreaderExtendedInfoHandler data to
the NavigationEntry in OnDistillComplete(). Since OnDistillComplete() is
called by the throttle, the navigation entry might not be persisted yet,
so the behavior is not defined. On backward navigations we would
sometimes persist to the previous navigation entry, causing bugs where
pages would incorrectly show up as readable. Since we wait for
DidFinishNavigation, we can safely get the last committed navgiation
entry.
  • Loading branch information
Kevin Kuehler committed Sep 2, 2021
1 parent bef2502 commit 1a34c05
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 6 deletions.
14 changes: 9 additions & 5 deletions browser/speedreader/speedreader_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,6 @@ void SpeedreaderTabHelper::HideBubble() {
}

void SpeedreaderTabHelper::OnDistillComplete() {
// Perform a state transition
auto* entry = web_contents()->GetController().GetLastCommittedEntry();
DCHECK(entry);

// Perform a state transition
if (distill_state_ == DistillState::kSpeedreaderModePending) {
distill_state_ = DistillState::kSpeedreaderMode;
Expand All @@ -217,8 +213,16 @@ void SpeedreaderTabHelper::OnDistillComplete() {
DCHECK(distill_state_ == DistillState::kSpeedreaderMode ||
distill_state_ == DistillState::kReaderMode);
}
}

void SpeedreaderTabHelper::DidFinishNavigation(
content::NavigationHandle* navigation_handle) {
if (!navigation_handle->HasCommitted()) {
distill_state_ = DistillState::kNone;
}
auto* entry = web_contents()->GetController().GetLastCommittedEntry();
DCHECK(entry);

// Save current distill state to navigation entry cache.
SpeedreaderExtendedInfoHandler::PersistMode(entry, distill_state_);
}

Expand Down
2 changes: 2 additions & 0 deletions browser/speedreader/speedreader_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class SpeedreaderTabHelper
content::NavigationHandle* navigation_handle) override;
void DidRedirectNavigation(
content::NavigationHandle* navigation_handle) override;
void DidFinishNavigation(
content::NavigationHandle* navigation_handle) override;

// SpeedreaderResultDelegate:
void OnDistillComplete() override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ void SpeedreaderExtendedInfoHandler::PersistMode(
kPageSavedSpeedreaderMode);
break;
default:
NOTREACHED();
return;
}

Expand Down

0 comments on commit 1a34c05

Please sign in to comment.