Use InterstitialPage through a delegate interface instead of deriving from it.

Use InterstitialPage through a delegate interface instead of deriving from it.

In a followup change I'll create an API around InterstitialPage.

BUG=98716
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=120840

[Avi (use Gerrit), 2012-02-06 19:27:05]

lgtm

http://codereview.chromium.org/9323071/diff/9029/chrome/browser/automation/te...
File chrome/browser/automation/testing_automation_provider.cc (right):

http://codereview.chromium.org/9323071/diff/9029/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:205: interstitial_page_
= InterstitialPage::Create(tab, true, url , this);
space after |url|

[jam, 2012-02-06 23:58:51]

Avi: I ended up having to change this a little since your review to fix tests.

The main difference is that I discovered that
InterstitialPage::Proceed/DontProceed are called by content (i.e. when user
navigates away), and so the code in the functions that you used to override them
in chrome weren't called. So instead now I have a callback to
InterstitialPageDelegates that tells them when a proceed/cancel happens.

[Avi (use Gerrit), 2012-02-07 00:33:50]

SLGTM

http://codereview.chromium.org/9323071/diff/18006/content/public/browser/inte...
File content/public/browser/interstitial_page_delegate.h (right):

http://codereview.chromium.org/9323071/diff/18006/content/public/browser/inte...
content/public/browser/interstitial_page_delegate.h:32: // Invoked when the page
sent a command through DOMAutomation.
I thought DOMAutomation was a chrome concept :)

[jam, 2012-02-07 01:10:05]

http://codereview.chromium.org/9323071/diff/18006/content/public/browser/inte...
File content/public/browser/interstitial_page_delegate.h (right):

http://codereview.chromium.org/9323071/diff/18006/content/public/browser/inte...
content/public/browser/interstitial_page_delegate.h:32: // Invoked when the page
sent a command through DOMAutomation.
On 2012/02/07 00:33:50, Avi wrote:
> I thought DOMAutomation was a chrome concept :)

the larger automation code is. But this unfortunately named class is used for
interstitials and not just automation. it probably needs to be renamed

[oshima, 2012-02-08 15:10:21]

John, OfflineLoadPageTest.OfflinePageProceed is crashing after this.
It's not your fault because the bot doesn't seem to be running unit_test
(probably regression from aura switch. I'll work on that), but I couldn't revert
probably because there are too many check ins. Could you please look into it?

http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS/b...


On 2012/02/07 01:10:05, John Abd-El-Malek wrote:
>
http://codereview.chromium.org/9323071/diff/18006/content/public/browser/inte...
> File content/public/browser/interstitial_page_delegate.h (right):
> 
>
http://codereview.chromium.org/9323071/diff/18006/content/public/browser/inte...
> content/public/browser/interstitial_page_delegate.h:32: // Invoked when the
page
> sent a command through DOMAutomation.
> On 2012/02/07 00:33:50, Avi wrote:
> > I thought DOMAutomation was a chrome concept :)
> 
> the larger automation code is. But this unfortunately named class is used for
> interstitials and not just automation. it probably needs to be renamed

[jam, 2012-02-08 21:56:39]

(just saw this)

is it crashing even after 120852?

On 2012/02/08 15:10:21, oshima wrote:
> John, OfflineLoadPageTest.OfflinePageProceed is crashing after this.
> It's not your fault because the bot doesn't seem to be running unit_test
> (probably regression from aura switch. I'll work on that), but I couldn't
revert
> probably because there are too many check ins. Could you please look into it?
> 
>
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%2520ChromiumOS...
> 
> 
> On 2012/02/07 01:10:05, John Abd-El-Malek wrote:
> >
>
http://codereview.chromium.org/9323071/diff/18006/content/public/browser/inte...
> > File content/public/browser/interstitial_page_delegate.h (right):
> > 
> >
>
http://codereview.chromium.org/9323071/diff/18006/content/public/browser/inte...
> > content/public/browser/interstitial_page_delegate.h:32: // Invoked when the
> page
> > sent a command through DOMAutomation.
> > On 2012/02/07 00:33:50, Avi wrote:
> > > I thought DOMAutomation was a chrome concept :)
> > 
> > the larger automation code is. But this unfortunately named class is used
for
> > interstitials and not just automation. it probably needs to be renamed

[jam, 2012-02-08 22:14:12]

nm, i looked through and saw that it is. i see the issue, just sent
http://codereview.chromium.org/9361047/ and will test locally

On 2012/02/08 21:56:39, John Abd-El-Malek wrote:
> (just saw this)
> 
> is it crashing even after 120852?
> 
> On 2012/02/08 15:10:21, oshima wrote:
> > John, OfflineLoadPageTest.OfflinePageProceed is crashing after this.
> > It's not your fault because the bot doesn't seem to be running unit_test
> > (probably regression from aura switch. I'll work on that), but I couldn't
> revert
> > probably because there are too many check ins. Could you please look into
it?