FrameLoader: add and export isReloadLoadType()

FrameLoader: add and export isReloadLoadType()

Now FrameLoader.h export isBackForwardLoadType(), but isReloadLoadType()
equivalent code exist here and there. Let me add isReloadLoadType() too
and export it to use from other places.

BUG=332602
Committed: https://crrev.com/af0efbea73ecde533d1577864982288fa53b96e5
Cr-Commit-Position: refs/heads/master@{#409742}

[Takashi Toyoshima, 2016-07-27 09:42:24]

Description was changed from

==========
FrameLoader: add and export isReloadLoadType()

Now FrameLoader.h export isBackForwardLoadType(), but isReloadLoadType()
equivalent code exist here and there. Let me add isReloadLoadType() too
and export it to use from other places.

BUG=332602
==========

to

==========
FrameLoader: add and export isReloadLoadType()

Now FrameLoader.h export isBackForwardLoadType(), but isReloadLoadType()
equivalent code exist here and there. Let me add isReloadLoadType() too
and export it to use from other places.

BUG=332602
==========

[Takashi Toyoshima, 2016-07-27 09:42:25]

toyoshim@chromium.org changed reviewers:
+ kinuko@chromium.org

[Takashi Toyoshima, 2016-07-27 10:26:49]

Can you take a look?
This is split from https://codereview.chromium.org/2167623002

https://codereview.chromium.org/2186863002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/LocalFrame.cpp (right):

https://codereview.chromium.org/2186863002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/LocalFrame.cpp:359:
DCHECK(isReloadLoadType(loadType));
This isn't mechanical change, but this should allow
FrameLoadTypeReloadMainResource.

https://codereview.chromium.org/2186863002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/LocalFrame.cpp:368: DCHECK(loadType ==
FrameLoadTypeReload);
For me, this DCHECK is easier to understand the intention than the original
ASSERT that I was confused a little at the first glance. What do you think?

Note: ClientRedirectPolicy is binary of NotClientRedirect or ClientRedirect.

https://codereview.chromium.org/2186863002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/2186863002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:113: return type ==
FrameLoadTypeBackForward
cl format did.

https://codereview.chromium.org/2186863002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:126: return type ==
FrameLoadTypeBackForward || isReloadLoadType(type);
This also should include ReloadMainResource.

[kinuko, 2016-07-27 12:25:57]

Looks reasonable change to me, but this looks to have possible behavior changes
and test failure looks relevant?

[Takashi Toyoshima, 2016-07-28 02:46:17]

Sorry for asking review before checking dbg run results.
I kicked new try of linux_precise_blink_dbg because linux_blink_* seem to be
turned down before I checked results :(

[Takashi Toyoshima, 2016-07-28 04:36:56]

Just in case, notes from layout test results for record.

virtual/threaded/fast/scroll-behavior/overflow-scroll-scrollTo.html
virtual/threaded/fast/scroll-behavior/smooth-scroll/main-thread-scrolling-reason-correctness.html
crash => STDERR: [1:3:0727/182649:4164017782:FATAL:scheduler.cc(160)] Check
failed: state_machine_.pending_swaps() > 0 (0 vs.
0){"compositor_timing_history":

http/tests/security/media-element-audio-source-node-cross-origin.html
diff => +CONSOLE ERROR: line 34: Uncaught (in promise) InvalidStateError: cannot
startRendering when an OfflineAudioContext is running

[Takashi Toyoshima, 2016-07-28 04:39:47]

https://codereview.chromium.org/2186863002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/2186863002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:126: return type ==
FrameLoadTypeBackForward || isReloadLoadType(type);
This is the only change that could change behavior.
Let me investigate this.

[Takashi Toyoshima, 2016-07-28 10:19:58]

I could not reproduce them on local machine.

https://build.chromium.org/p/tryserver.blink/builders/linux_precise_blink_dbg
It seems same tests are failing for other developers, e.g.
https://build.chromium.org/p/tryserver.blink/builders/linux_precise_blink_dbg...

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20(dbg)
Also they look flaky even on the trunk waterfall.

Let me post try jobs for other platforms.

[Takashi Toyoshima, 2016-07-28 10:20:17]

The CQ bit was checked by toyoshim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 10:20:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-28 11:31:54]

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

[commit-bot: I haz the power, 2016-07-28 11:31:54]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Takashi Toyoshima, 2016-07-29 11:15:17]

New try of linux_precise_blink_dbg passed, but linux_android_rel_ng failed
again. Let me take a look.

[Takashi Toyoshima, 2016-07-29 12:29:28]

In my Nexus4, failed four tests could not pass even without my patch, but
because of different reason.

FYI, these are failed four tests of android_webview_test_apk.
org.chromium.android_webview.test.AwSettingsTest#testLoadWithOverviewModeWithTwoViews
org.chromium.android_webview.test.AwSettingsTest#testLoadWithOverviewModeViewportTagWithTwoViews
org.chromium.android_webview.test.AwSettingsTest#testLoadWithOverviewModeWithTwoViews
with {--webview-sandboxed-renderer}
org.chromium.android_webview.test.AwSettingsTest#testLoadWithOverviewModeViewportTagWithTwoViews
with {--webview-sandboxed-renderer}

Failure reason:
C   40.015s Main  [FAIL]
org.chromium.android_webview.test.AwSettingsTest#testLoadWithOverviewModeViewportTagWithTwoViews
with {--webview-sandboxed-renderer}:
C   40.015s Main  junit.framework.AssertionFailedError: expected:<1.0> but
was:<0.25>
C   40.016s Main        at
org.chromium.android_webview.test.AwSettingsTest$AwSettingsLoadWithOverviewModeTestHelper.doEnsureSettingHasValue(AwSettingsTest.java:1348)
C   40.016s Main        at
org.chromium.android_webview.test.AwSettingsTest$AwSettingsLoadWithOverviewModeTestHelper.doEnsureSettingHasValue(AwSettingsTest.java:1298)
C   40.016s Main        at
org.chromium.android_webview.test.AwSettingsTest$AwSettingsTestHelper.ensureSettingHasValue(AwSettingsTest.java:144)
C   40.016s Main        at
org.chromium.android_webview.test.AwSettingsTest$AwSettingsTestHelper.ensureSettingHasInitialValue(AwSettingsTest.java:88)
C   40.016s Main        at
org.chromium.android_webview.test.AwSettingsTest.runPerViewSettingsTest(AwSettingsTest.java:2993)
C   40.016s Main        at
org.chromium.android_webview.test.AwSettingsTest.testLoadWithOverviewModeViewportTagWithTwoViews(AwSettingsTest.java:2672)
C   40.016s Main        at
android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
C   40.016s Main        at
android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
C   40.016s Main        at
android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192)
C   40.017s Main        at
org.chromium.base.test.BaseTestResult.runParameterized(BaseTestResult.java:161)
C   40.017s Main        at
org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:124)
C   40.017s Main        at
android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191)
C   40.017s Main        at
android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176)
C   40.017s Main        at
android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555)
C   40.017s Main        at
android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1853)

I'd speculatively revert a part that may change behavior, and see what happens
on try.

[Takashi Toyoshima, 2016-07-29 13:39:31]

Kinuko-san, I partially reverted the behavior change from this CL, and it works
fine.
What do you think about the Patch Set 3?

This might be related to the original CL's failures, and I will investigate what
happens there.
FYI, the original CL means https://codereview.chromium.org/2167623002

[Takashi Toyoshima, 2016-07-29 13:40:30]

The CQ bit was checked by toyoshim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 13:40:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 14:50:49]

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

[commit-bot: I haz the power, 2016-07-29 14:50:51]

Dry run: This issue passed the CQ dry run.

[kinuko, 2016-08-01 17:03:31]

ok, lgtm. (sorry for slow review)

[Takashi Toyoshima, 2016-08-02 08:32:47]

toyoshim@chromium.org changed reviewers:
+ tkent@chromium.org

[Takashi Toyoshima, 2016-08-02 08:32:48]

+tkent@chromium.org for Source/web

[tkent, 2016-08-02 23:12:14]

lgtm

https://codereview.chromium.org/2186863002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/LocalFrame.cpp (right):

https://codereview.chromium.org/2186863002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/LocalFrame.cpp:368: DCHECK(loadType ==
FrameLoadTypeReload);
DCHECK -> DCHECK_EQ

[Takashi Toyoshima, 2016-08-03 04:31:25]

https://codereview.chromium.org/2186863002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/LocalFrame.cpp (right):

https://codereview.chromium.org/2186863002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/LocalFrame.cpp:368: DCHECK(loadType ==
FrameLoadTypeReload);
On 2016/08/02 23:12:14, tkent wrote:
> DCHECK -> DCHECK_EQ

Done.

[Takashi Toyoshima, 2016-08-03 04:31:31]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2016-08-03 04:31:31]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
kinuko@chromium.org
Link to the patchset: https://codereview.chromium.org/2186863002/#ps60001
(title: "review #23")

[commit-bot: I haz the power, 2016-08-03 04:31:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 05:41:45]

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

[commit-bot: I haz the power, 2016-08-03 05:41:46]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Takashi Toyoshima, 2016-08-04 04:47:12]

The CQ bit was checked by toyoshim@chromium.org

[commit-bot: I haz the power, 2016-08-04 04:47:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-04 06:01:52]

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

[commit-bot: I haz the power, 2016-08-04 06:01:53]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Takashi Toyoshima, 2016-08-04 06:19:54]

Patchset #5 (id:80001) has been deleted

[Takashi Toyoshima, 2016-08-04 07:28:18]

Oops, I mistakenly reverted the fix of PS3 in the PS4, and failed in CQ.
PS5 should pass!

[Takashi Toyoshima, 2016-08-04 07:28:23]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2016-08-04 07:28:24]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
kinuko@chromium.org
Link to the patchset: https://codereview.chromium.org/2186863002/#ps100001
(title: "revert of wrong revert")

[commit-bot: I haz the power, 2016-08-04 07:28:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-04 08:36:04]

Description was changed from

==========
FrameLoader: add and export isReloadLoadType()

Now FrameLoader.h export isBackForwardLoadType(), but isReloadLoadType()
equivalent code exist here and there. Let me add isReloadLoadType() too
and export it to use from other places.

BUG=332602
==========

to

==========
FrameLoader: add and export isReloadLoadType()

Now FrameLoader.h export isBackForwardLoadType(), but isReloadLoadType()
equivalent code exist here and there. Let me add isReloadLoadType() too
and export it to use from other places.

BUG=332602
==========

[commit-bot: I haz the power, 2016-08-04 08:36:05]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-08-04 08:39:11]

Description was changed from

==========
FrameLoader: add and export isReloadLoadType()

Now FrameLoader.h export isBackForwardLoadType(), but isReloadLoadType()
equivalent code exist here and there. Let me add isReloadLoadType() too
and export it to use from other places.

BUG=332602
==========

to

==========
FrameLoader: add and export isReloadLoadType()

Now FrameLoader.h export isBackForwardLoadType(), but isReloadLoadType()
equivalent code exist here and there. Let me add isReloadLoadType() too
and export it to use from other places.

BUG=332602

Committed: https://crrev.com/af0efbea73ecde533d1577864982288fa53b96e5
Cr-Commit-Position: refs/heads/master@{#409742}
==========

[commit-bot: I haz the power, 2016-08-04 08:39:13]

Patchset 5 (id:??) landed as
https://crrev.com/af0efbea73ecde533d1577864982288fa53b96e5
Cr-Commit-Position: refs/heads/master@{#409742}