Add TabWebContentsTracker

chrome/browser/managed_mode/managed_mode_navigation_observer.cc

 // Copyright (c) 2012 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/managed_mode/managed_mode_navigation_observer.h"
 
 #include "base/bind.h"
 #include "base/i18n/rtl.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
@@ skipping 238 matching lines @@
   ManagedModeNavigationObserver* observer =
       ManagedModeNavigationObserver::FromWebContents(web_contents());
   observer->PreviewInfobarDismissed();
 }
 
 }  // namespace
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(ManagedModeNavigationObserver);
 
 ManagedModeNavigationObserver::~ManagedModeNavigationObserver() {
+  TabWebContentsTracker::RemoveObserver(this);
   RemoveTemporaryException();
 }
 
 ManagedModeNavigationObserver::ManagedModeNavigationObserver(
     content::WebContents* web_contents)
     : WebContentsObserver(web_contents),
       warn_infobar_delegate_(NULL),
       preview_infobar_delegate_(NULL),
       state_(RECORDING_URLS_BEFORE_PREVIEW),
       is_elevated_(false),
       last_allowed_page_(-1),
+      observer_state_cleared_(false),
       finished_redirects_(false) {
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
   managed_user_service_ = ManagedUserServiceFactory::GetForProfile(profile);
   if (!managed_user_service_->ProfileIsManaged())
     is_elevated_ = true;
   url_filter_ = managed_user_service_->GetURLFilterForUIThread();
+  TabWebContentsTracker::AddObserver(this);
+}
+
+void ManagedModeNavigationObserver::OnTabReplacedAt(
+    TabStripModel* tab_strip_model,
+    content::WebContents* old_contents,
+    content::WebContents* new_contents,
+    int index) {
+  if (new_contents == web_contents()) {

[Bernhard Bauer, 2013/04/26 16:30:10]

Early-return otherwise?

+    // First, copy over some data of the old navigation observer. This only

[Bernhard Bauer, 2013/04/26 16:30:10]

This seems...brittle. I'm okay with it, but I could imagine running into subtle
bugs down the line later (we already need special treatment when
ClearObserverState has been called, for example). I think an easier way would be
to add a ReleaseUserData() method to base::SupportsUserData and move the whole
object over.

[Adrian Kuegel, 2013/04/29 08:11:15]

But if I copy the whole object over, it would still not contain the right data,
and I would have to do the same thing. This replace of WebContents seems to
happen mostly (if not always) in the case where a page is prerendered. In that
case, we will be notified after the page has been already prerendered, and then
the ManagedModeNavigationObserver attached to the prerendered WebContents
contains some data we may need to copy over (or contains the information that we
have to clear some data). If we really just want to copy the object over, we
would have to make sure that we do that when creating the WebContents that is
used for the prerendering. I already thought that this would probably be the
best thing, but I am not sure if that is feasible (we would have to clone the
object, because I think we cannot be sure if the prerendered WebContents will
actually be used later).

[Bernhard Bauer, 2013/04/29 08:39:38]

Like I said, I'm okay with this, I just want to keep us from having to add more
special cases later. Seeing as the prerender-swap-in happens instead of a
navigation, could we get the desired behavior by treating it as a navigation
event (recording URLs etc.)?

+    // applies if the ClearObserverState function was not called on the new

[Adrian Kuegel, 2013/04/26 15:52:27]

Reassigning the ManagedModeNavigationObserver to another WebContents does not
work. When removing from WebContentsUserData it would already call the
destructor.

+    // navigation observer, because that would reset the data we are about to
+    // copy.
+    if (!observer_state_cleared_) {
+      ManagedModeNavigationObserver* observer =
+          ManagedModeNavigationObserver::FromWebContents(old_contents);
+      navigated_urls_.insert(observer->navigated_urls_.begin(),
+                             observer->navigated_urls_.end());
+      if (last_allowed_page_ < 0)
+        last_allowed_page_ = observer->last_allowed_page_;
+      if (observer->warn_infobar_delegate_ && !warn_infobar_delegate_) {
+        warn_infobar_delegate_ = ManagedModeWarningInfobarDelegate::Create(
+            InfoBarService::FromWebContents(web_contents()),
+            last_allowed_page_);
+      }
+      if (observer->preview_infobar_delegate_ && !preview_infobar_delegate_) {
+        preview_infobar_delegate_ = ManagedModePreviewInfobarDelegate::Create(

[Bernhard Bauer, 2013/04/26 16:30:10]

Doesn't InfobarTabHelper or something similar take care of this? At least I've
always seen Infobars come over even when a WebContents is replaced.

[Adrian Kuegel, 2013/04/29 08:11:15]

Yes, that is possible. I guess I can remove that code part then.

+            InfoBarService::FromWebContents(web_contents()));
+      }
+      // We don't need to copy |state_| and |finished_redirects_|, because that
+      // should be up-to-date.
+    }
+  }
 }
 
 void ManagedModeNavigationObserver::AddTemporaryException() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(web_contents());
 
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
       base::Bind(&ManagedModeResourceThrottle::AddTemporaryException,
@@ skipping 93 matching lines @@
          StartsWithASCII(url, extension_urls::kGalleryBrowsePrefix, false);
 }
 
 void ManagedModeNavigationObserver::ClearObserverState() {
   if (preview_infobar_delegate_) {
     InfoBarService* infobar_service =
         InfoBarService::FromWebContents(web_contents());
     infobar_service->RemoveInfoBar(preview_infobar_delegate_);
     preview_infobar_delegate_ = NULL;
   }
+  observer_state_cleared_ = true;

[Bernhard Bauer, 2013/04/26 16:30:10]

So you set this to true when we have cleared observer state at some point, and
then never set it to false again?

[Adrian Kuegel, 2013/04/29 08:11:15]

Yes, but that is exactly as expected. This variable is only used for deciding if
to copy data from the old ManagedModeNavigationObserver. We only need to copy
data from the old ManagedModeNavigationObserver, if _never_ ClearObserverState()
was called in the new Observer. Imagine we had copied the data over when
initializing the new Observer (which I think we should actually do). Then this
data would have been gone now.

   navigated_urls_.clear();
   last_url_ = GURL();
   state_ = RECORDING_URLS_BEFORE_PREVIEW;
   // TODO(sergiu): Remove these logging calls once this is stable.
   DVLOG(1) << "Clearing observer state";
   RemoveTemporaryException();
 }
 
 void ManagedModeNavigationObserver::NavigateToPendingEntry(
     const GURL& url,
@@ skipping 167 matching lines @@
   HistoryService* history_service =
      HistoryServiceFactory::GetForProfile(profile, Profile::IMPLICIT_ACCESS);
 
   // |history_service| is null if saving history is disabled.
   if (history_service)
     history_service->AddPage(add_page_args);
 
   // Show the interstitial.
   new ManagedModeInterstitial(web_contents, url, callback);
 }