Test for the fix for history.replaceState navigations submitted earlier.

Unit Test for the fix for history.replaceState navigations submitted earlier.

Unit test for the fix submitted with https://codereview.chromium.org/316553007

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

[mfomitchev, 2014-06-10 21:13:50]

@nasko - can you please take the first look?

[nasko, 2014-06-11 16:35:39]

It isn't ideal that the test is aura specific and it looks like
ScreenshotTracker can be extracted to a common location and reused in other
test. Considering though that all the other tests for screenshotting are in the
same file, it is reasonable to just add it. It might be good to file a bug to
extract those tests to not be aura specific, which will be a good starter bug
for new contributors.

All that said, LGTM with a nit.

https://codereview.chromium.org/316403004/diff/20001/content/browser/web_cont...
File content/browser/web_contents/web_contents_view_aura_browsertest.cc (right):

https://codereview.chromium.org/316403004/diff/20001/content/browser/web_cont...
content/browser/web_contents/web_contents_view_aura_browsertest.cc:577:
EXPECT_EQ(NULL, screenshot_manager()->screenshot_taken_for());
nit: for completeness, I'd add a real navigation or a pushState and check that a
screenshot is indeed taken.

[nasko, 2014-06-11 16:36:24]

On 2014/06/11 16:35:39, nasko wrote:
> It isn't ideal that the test is aura specific and it looks like
> ScreenshotTracker can be extracted to a common location and reused in other
> test. Considering though that all the other tests for screenshotting are in
the
> same file, it is reasonable to just add it. It might be good to file a bug to
> extract those tests to not be aura specific, which will be a good starter bug
> for new contributors.
> 
> All that said, LGTM with a nit.
> 
>
https://codereview.chromium.org/316403004/diff/20001/content/browser/web_cont...
> File content/browser/web_contents/web_contents_view_aura_browsertest.cc
(right):
> 
>
https://codereview.chromium.org/316403004/diff/20001/content/browser/web_cont...
> content/browser/web_contents/web_contents_view_aura_browsertest.cc:577:
> EXPECT_EQ(NULL, screenshot_manager()->screenshot_taken_for());
> nit: for completeness, I'd add a real navigation or a pushState and check that
a
> screenshot is indeed taken.

Also, the title of the issue is misleading, as this is a browser test, not an
unit test.

[mfomitchev, 2014-06-11 17:22:01]

+sky for owner review

> It isn't ideal that the test is aura specific and it looks like
> ScreenshotTracker can be extracted to a common location and reused in other
> test. Considering though that all the other tests for screenshotting are in
the
> same file, it is reasonable to just add it. It might be good to file a bug to
> extract those tests to not be aura specific, which will be a good starter bug
> for new contributors.

As it stands, currently the screenshots are only taken on aura builds because
overscroll is only enabled on aura builds.
      if (delegate_->CanOverscrollContent())
        controller_->TakeScreenshot();

> Also, the title of the issue is misleading, as this is a browser test, not an
> unit test.

Updated

[mfomitchev, 2014-06-11 17:22:07]

https://codereview.chromium.org/316403004/diff/20001/content/browser/web_cont...
File content/browser/web_contents/web_contents_view_aura_browsertest.cc (right):

https://codereview.chromium.org/316403004/diff/20001/content/browser/web_cont...
content/browser/web_contents/web_contents_view_aura_browsertest.cc:577:
EXPECT_EQ(NULL, screenshot_manager()->screenshot_taken_for());
On 2014/06/11 16:35:39, nasko wrote:
> nit: for completeness, I'd add a real navigation or a pushState and check that
a
> screenshot is indeed taken.

Done.

[mfomitchev, 2014-06-11 17:22:29]

+sky for owner reveiw

[sky, 2014-06-11 19:38:08]

LGTM

[mfomitchev, 2014-06-11 20:10:30]

The CQ bit was checked by mfomitchev@chromium.org

[commit-bot: I haz the power, 2014-06-11 20:12:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mfomitchev@chromium.org/316403004/40001

[commit-bot: I haz the power, 2014-06-11 21:30:52]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15604)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18742)

[commit-bot: I haz the power, 2014-06-11 21:34:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-11 21:34:02]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15662)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18797)

[mfomitchev, 2014-06-11 21:34:40]

The CQ bit was checked by mfomitchev@chromium.org

[commit-bot: I haz the power, 2014-06-11 21:37:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mfomitchev@chromium.org/316403004/40001

[commit-bot: I haz the power, 2014-06-11 22:15:55]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15678)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18812)

[commit-bot: I haz the power, 2014-06-11 22:18:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-11 22:18:22]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15710)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18843)

[mfomitchev, 2014-06-12 14:45:41]

The CQ bit was checked by mfomitchev@chromium.org

[commit-bot: I haz the power, 2014-06-12 14:47:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mfomitchev@chromium.org/316403004/40001

[commit-bot: I haz the power, 2014-06-12 19:31:23]

Change committed as 276770