Add an empty boolean to WebFrameClient::didFinishDocumentLoad

Add an empty boolean to WebFrameClient::didFinishDocumentLoad

This will be used by chromium for error message policy decisions.

Internally, change DocumentLoader to use an enum-based state machine. There were
already bools for whether loading is in progress or whether the DocumentLoader
has committed, and adding another bool for whether real data has been received
seems silly.

BUG=331745
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198817

[Nate Chapin, 2015-07-10 18:11:33]

japhet@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2015-07-13 20:53:56]

https://codereview.chromium.org/1229263003/diff/40001/Source/core/loader/Docu...
File Source/core/loader/DocumentLoader.h (right):

https://codereview.chromium.org/1229263003/diff/40001/Source/core/loader/Docu...
Source/core/loader/DocumentLoader.h:105: bool isCommittedButEmpty() const {
return m_state == Committed; }
Does this imply that mainReceivedError/finishedLoading aren't called in this
case?

https://codereview.chromium.org/1229263003/diff/40001/Source/web/FrameLoaderC...
File Source/web/FrameLoaderClientImpl.cpp (right):

https://codereview.chromium.org/1229263003/diff/40001/Source/web/FrameLoaderC...
Source/web/FrameLoaderClientImpl.cpp:376: void
FrameLoaderClientImpl::dispatchDidFinishDocumentLoad(bool empty)
I feel like empty isn't quite descriptive enough. An enum (though that's kind of
annoying to plumb from core -> web -> content) or a name like emptyResponseBody
might help.

[Nate Chapin, 2015-07-13 21:13:28]

https://codereview.chromium.org/1229263003/diff/40001/Source/core/loader/Docu...
File Source/core/loader/DocumentLoader.h (right):

https://codereview.chromium.org/1229263003/diff/40001/Source/core/loader/Docu...
Source/core/loader/DocumentLoader.h:105: bool isCommittedButEmpty() const {
return m_state == Committed; }
On 2015/07/13 20:53:56, dcheng wrote:
> Does this imply that mainReceivedError/finishedLoading aren't called in this
> case?

The most common case would be that this is called when finishedLoading() is on
the stack but not finished, inside finishedLoading()'s endWriting() call.

https://codereview.chromium.org/1229263003/diff/40001/Source/web/FrameLoaderC...
File Source/web/FrameLoaderClientImpl.cpp (right):

https://codereview.chromium.org/1229263003/diff/40001/Source/web/FrameLoaderC...
Source/web/FrameLoaderClientImpl.cpp:376: void
FrameLoaderClientImpl::dispatchDidFinishDocumentLoad(bool empty)
On 2015/07/13 20:53:56, dcheng wrote:
> I feel like empty isn't quite descriptive enough. An enum (though that's kind
of
> annoying to plumb from core -> web -> content) or a name like
emptyResponseBody
> might help.

I changed the chromium-side to use the name document_is_empty. Is the
blink-styled equivalent OK?

[dcheng, 2015-07-13 21:23:22]

https://codereview.chromium.org/1229263003/diff/40001/Source/web/FrameLoaderC...
File Source/web/FrameLoaderClientImpl.cpp (right):

https://codereview.chromium.org/1229263003/diff/40001/Source/web/FrameLoaderC...
Source/web/FrameLoaderClientImpl.cpp:376: void
FrameLoaderClientImpl::dispatchDidFinishDocumentLoad(bool empty)
On 2015/07/13 at 21:13:28, Nate Chapin wrote:
> On 2015/07/13 20:53:56, dcheng wrote:
> > I feel like empty isn't quite descriptive enough. An enum (though that's
kind of
> > annoying to plumb from core -> web -> content) or a name like
emptyResponseBody
> > might help.
> 
> I changed the chromium-side to use the name document_is_empty. Is the
blink-styled equivalent OK?

Yes, that seems better.

[Nate Chapin, 2015-07-13 21:31:22]

On 2015/07/13 21:23:22, dcheng wrote:
>
https://codereview.chromium.org/1229263003/diff/40001/Source/web/FrameLoaderC...
> File Source/web/FrameLoaderClientImpl.cpp (right):
> 
>
https://codereview.chromium.org/1229263003/diff/40001/Source/web/FrameLoaderC...
> Source/web/FrameLoaderClientImpl.cpp:376: void
> FrameLoaderClientImpl::dispatchDidFinishDocumentLoad(bool empty)
> On 2015/07/13 at 21:13:28, Nate Chapin wrote:
> > On 2015/07/13 20:53:56, dcheng wrote:
> > > I feel like empty isn't quite descriptive enough. An enum (though that's
> kind of
> > > annoying to plumb from core -> web -> content) or a name like
> emptyResponseBody
> > > might help.
> > 
> > I changed the chromium-side to use the name document_is_empty. Is the
> blink-styled equivalent OK?
> 
> Yes, that seems better.

Done.

[dcheng, 2015-07-13 21:40:38]

lgtm

[Nate Chapin, 2015-07-13 22:26:51]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2015-07-13 22:27:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1229263003/60001

[commit-bot: I haz the power, 2015-07-13 23:53:32]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198817