Commit 4973b542 authored by Matt Falkenhagen's avatar Matt Falkenhagen Committed by Commit Bot

prerender: Simplify some dead code around SetIsPrerendering().

* PrerenderRenderFrameObserver::SetIsPrerendering() was only calling
  PrerenderHelper::SetIsPrerendering() when mode != kNoPrerender,
  and the function just early returns in that case. Therefore, delete
  PrerenderHelper::SetIsPrerendering().
* Also mark some code only used by tests as test-only code.

This is a bit uncomfortable since we might be needing this code
as prerender gets revisited. However, I found it confusing that the
caller was calling the callee only when the callee ignores the call,
so think it's constructive to simplify this.

This also reveals that PrerenderURLLoaderThrottle::PrerenderUsed() is
never called, but I won't traverse into there since we might want the
code.

Bug: 755921

Change-Id: I486e78066350c8a20c63fdca3feb1344766c6b15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2396057
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarRobert Ogden <robertogden@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805225}
parent 97fc7242
...@@ -200,7 +200,7 @@ class UnitTestPrerenderManager : public PrerenderManager { ...@@ -200,7 +200,7 @@ class UnitTestPrerenderManager : public PrerenderManager {
prerender_data->ReleaseContents(); prerender_data->ReleaseContents();
active_prerenders_.erase(to_erase); active_prerenders_.erase(to_erase);
prerender_contents->PrepareForUse(); prerender_contents->MarkAsUsedForTesting();
return prerender_contents; return prerender_contents;
} }
......
...@@ -587,7 +587,7 @@ std::unique_ptr<base::DictionaryValue> PrerenderContents::GetAsValue() const { ...@@ -587,7 +587,7 @@ std::unique_ptr<base::DictionaryValue> PrerenderContents::GetAsValue() const {
return dict_value; return dict_value;
} }
void PrerenderContents::PrepareForUse() { void PrerenderContents::MarkAsUsedForTesting() {
SetFinalStatus(FINAL_STATUS_USED); SetFinalStatus(FINAL_STATUS_USED);
if (prerender_contents_.get()) { if (prerender_contents_.get()) {
......
...@@ -204,8 +204,10 @@ class PrerenderContents : public content::NotificationObserver, ...@@ -204,8 +204,10 @@ class PrerenderContents : public content::NotificationObserver,
std::unique_ptr<base::DictionaryValue> GetAsValue() const; std::unique_ptr<base::DictionaryValue> GetAsValue() const;
// Marks prerender as used and releases any throttled resource requests. // This function is not currently called in production since prerendered
void PrepareForUse(); // contents are never used (only prefetch is supported), but it may be used in
// the future: https://crbug.com/1126305
void MarkAsUsedForTesting();
// Increments the number of bytes fetched over the network for this prerender. // Increments the number of bytes fetched over the network for this prerender.
void AddNetworkBytes(int64_t bytes); void AddNetworkBytes(int64_t bytes);
......
...@@ -49,7 +49,7 @@ std::unique_ptr<blink::URLLoaderThrottle> PrerenderHelper::MaybeCreateThrottle( ...@@ -49,7 +49,7 @@ std::unique_ptr<blink::URLLoaderThrottle> PrerenderHelper::MaybeCreateThrottle(
auto throttle = std::make_unique<PrerenderURLLoaderThrottle>( auto throttle = std::make_unique<PrerenderURLLoaderThrottle>(
prerender_helper->prerender_mode(), prerender_helper->histogram_prefix(), prerender_helper->prerender_mode(), prerender_helper->histogram_prefix(),
std::move(canceler)); std::move(canceler));
prerender_helper->AddThrottle(throttle->AsWeakPtr()); prerender_helper->AddThrottle(*throttle);
return throttle; return throttle;
} }
...@@ -85,39 +85,16 @@ void PrerenderHelper::OnDestruct() { ...@@ -85,39 +85,16 @@ void PrerenderHelper::OnDestruct() {
delete this; delete this;
} }
void PrerenderHelper::SetIsPrerendering(prerender::mojom::PrerenderMode mode, void PrerenderHelper::AddThrottle(PrerenderURLLoaderThrottle& throttle) {
const std::string& histogram_prefix) {
// Immediately after construction, |this| may receive the message that
// triggered its creation. If so, ignore it.
if (mode != prerender::mojom::PrerenderMode::kNoPrerender)
return;
std::vector<base::WeakPtr<PrerenderURLLoaderThrottle>> throttles =
std::move(throttles_);
// |this| must be deleted so PrerenderHelper::IsPrerendering returns false
// when the visibility state is updated, so the visibility state string will
// not be "prerendered".
delete this;
for (auto& throttle : throttles) {
if (throttle)
throttle->PrerenderUsed();
}
}
void PrerenderHelper::AddThrottle(
const base::WeakPtr<PrerenderURLLoaderThrottle>& throttle) {
// Keep track of how many pending throttles we have, as we want to defer // Keep track of how many pending throttles we have, as we want to defer
// sending the "prefetch finished" signal until they are destroyed. This is // sending the "prefetch finished" signal until they are destroyed. This is
// important since that signal tells the browser that it can tear down this // important since that signal tells the browser that it can tear down this
// renderer which could interrupt subresource prefetching. // renderer which could interrupt subresource prefetching.
if (prerender_mode_ == prerender::mojom::PrerenderMode::kPrefetchOnly) { if (prerender_mode_ == prerender::mojom::PrerenderMode::kPrefetchOnly) {
prefetch_count_++; prefetch_count_++;
throttle->set_destruction_closure(base::BindOnce( throttle.set_destruction_closure(base::BindOnce(
&PrerenderHelper::OnThrottleDestroyed, weak_factory_.GetWeakPtr())); &PrerenderHelper::OnThrottleDestroyed, weak_factory_.GetWeakPtr()));
} }
throttles_.push_back(throttle);
} }
void PrerenderHelper::OnThrottleDestroyed() { void PrerenderHelper::OnThrottleDestroyed() {
......
...@@ -48,24 +48,18 @@ class PrerenderHelper ...@@ -48,24 +48,18 @@ class PrerenderHelper
} }
std::string histogram_prefix() const { return histogram_prefix_; } std::string histogram_prefix() const { return histogram_prefix_; }
void SetIsPrerendering(prerender::mojom::PrerenderMode mode,
const std::string& histogram_prefix);
private: private:
// RenderFrameObserver implementation. // RenderFrameObserver implementation.
void DidFinishDocumentLoad() override; void DidFinishDocumentLoad() override;
void OnDestruct() override; void OnDestruct() override;
void AddThrottle(const base::WeakPtr<PrerenderURLLoaderThrottle>& throttle); void AddThrottle(PrerenderURLLoaderThrottle& throttle);
void OnThrottleDestroyed(); void OnThrottleDestroyed();
void SendPrefetchFinished(); void SendPrefetchFinished();
const prerender::mojom::PrerenderMode prerender_mode_; const prerender::mojom::PrerenderMode prerender_mode_;
std::string histogram_prefix_; std::string histogram_prefix_;
// Pending requests for this frame..
std::vector<base::WeakPtr<PrerenderURLLoaderThrottle>> throttles_;
int prefetch_count_ = 0; int prefetch_count_ = 0;
bool prefetch_finished_ = false; bool prefetch_finished_ = false;
base::TimeTicks start_time_; base::TimeTicks start_time_;
......
...@@ -49,12 +49,10 @@ void PrerenderRenderFrameObserver::SetIsPrerendering( ...@@ -49,12 +49,10 @@ void PrerenderRenderFrameObserver::SetIsPrerendering(
prerender_helper = new prerender::PrerenderHelper(render_frame(), mode, prerender_helper = new prerender::PrerenderHelper(render_frame(), mode,
histogram_prefix); histogram_prefix);
} }
prerender_helper->SetIsPrerendering(mode, histogram_prefix);
} }
prerender::PrerenderObserverList::SetIsPrerenderingForFrame(render_frame(), prerender::PrerenderObserverList::SetIsPrerenderingForFrame(render_frame(),
is_prerendering); is_prerendering);
} }
} // namespace prerender } // namespace prerender
\ No newline at end of file
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment