Avoid showing the supervised user block interstitial more than once

chrome/browser/supervised_user/supervised_user_interstitial.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/supervised_user/supervised_user_interstitial.h"
 
 #include <stddef.h>
 
 #include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 100 matching lines @@
 // static
 void SupervisedUserInterstitial::Show(
     WebContents* web_contents,
     const GURL& url,
     supervised_user_error_page::FilteringBehaviorReason reason,
     bool initial_page_load,
     const base::Callback<void(bool)>& callback) {
   SupervisedUserInterstitial* interstitial = new SupervisedUserInterstitial(
       web_contents, url, reason, initial_page_load, callback);
 
-  // If Init() does not complete fully, immediately delete the interstitial.
-  if (!interstitial->Init())
-    delete interstitial;
-  // Otherwise |interstitial_page_| is responsible for deleting it.
+  // |interstitial_page_| is responsible for deleting the interstitial.
+  interstitial->Init();

[Marc Treib, 2017/05/02 08:24:47]

Should Init() now be merged into the ctor?

[Bernhard Bauer, 2017/05/02 09:02:53]

I could do that, but then the constructor would do quite a lot of things,
including handing off a pointer to itself to the interstitial page (which then
owns it, so it could decide to immediately destroy the delegate!), and that all
seems a bit brittle...

 }
 
 SupervisedUserInterstitial::SupervisedUserInterstitial(
     WebContents* web_contents,
     const GURL& url,
     supervised_user_error_page::FilteringBehaviorReason reason,
     bool initial_page_load,
     const base::Callback<void(bool)>& callback)
     : web_contents_(web_contents),
       profile_(Profile::FromBrowserContext(web_contents->GetBrowserContext())),
       interstitial_page_(NULL),
       url_(url),
       reason_(reason),
       initial_page_load_(initial_page_load),
       callback_(callback),
       weak_ptr_factory_(this) {}
 
 SupervisedUserInterstitial::~SupervisedUserInterstitial() {
   DCHECK(!web_contents_);
 }
 
-bool SupervisedUserInterstitial::Init() {
-  if (ShouldProceed()) {
-    // It can happen that the site was only allowed very recently and the URL
-    // filter on the IO thread had not been updated yet. Proceed with the
-    // request without showing the interstitial.
-    DispatchContinueRequest(true);
-    return false;
-  }
+void SupervisedUserInterstitial::Init() {
+  DCHECK(!ShouldProceed());
 
   InfoBarService* service = InfoBarService::FromWebContents(web_contents_);
   if (service) {
     // Remove all the infobars which are attached to |web_contents_| and for
     // which ShouldExpire() returns true.
     content::LoadCommittedDetails details;
     // |details.is_same_page| is default false, and |details.is_main_frame| is
     // default true. This results in is_navigation_to_different_page() returning
     // true.
     DCHECK(details.is_navigation_to_different_page());
@@ skipping 14 matching lines @@
     }
   }
 
   SupervisedUserService* supervised_user_service =
       SupervisedUserServiceFactory::GetForProfile(profile_);
   supervised_user_service->AddObserver(this);
 
   interstitial_page_ = content::InterstitialPage::Create(
       web_contents_, initial_page_load_, url_, this);
   interstitial_page_->Show();
-
-  return true;
 }
 
 // static
 std::string SupervisedUserInterstitial::GetHTMLContents(
     Profile* profile,
     supervised_user_error_page::FilteringBehaviorReason reason) {
   bool is_child_account = profile->IsChild();
 
   SupervisedUserService* supervised_user_service =
       SupervisedUserServiceFactory::GetForProfile(profile);
@@ skipping 30 matching lines @@
     NTP,
     ACCESS_REQUEST,
     HISTOGRAM_BOUNDING_VALUE
   };
 
   if (command == "\"back\"") {
     UMA_HISTOGRAM_ENUMERATION("ManagedMode.BlockingInterstitialCommand",
                               BACK,
                               HISTOGRAM_BOUNDING_VALUE);
 
-    DCHECK(web_contents_->GetController().GetTransientEntry());
     interstitial_page_->DontProceed();
     return;
   }
 
   if (command == "\"request\"") {
     UMA_HISTOGRAM_ENUMERATION("ManagedMode.BlockingInterstitialCommand",
                               ACCESS_REQUEST,
                               HISTOGRAM_BOUNDING_VALUE);
 
     SupervisedUserService* supervised_user_service =
@@ skipping 90 matching lines @@
 
   TabCloser::MaybeClose(web_contents_);
 }
 
 void SupervisedUserInterstitial::DispatchContinueRequest(
     bool continue_request) {
   SupervisedUserService* supervised_user_service =
       SupervisedUserServiceFactory::GetForProfile(profile_);
   supervised_user_service->RemoveObserver(this);
 
-  if (!callback_.is_null())
-    callback_.Run(continue_request);
+  callback_.Run(continue_request);
 
   // After this, the WebContents may be destroyed. Make sure we don't try to use
   // it again.
   web_contents_ = nullptr;
 }