[webkit-reviews] review denied: [Bug 39695] FrameLoader cleanup: Merge some bool members into a state machine : [Attachment 57906] Fix chromium compile issue

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Nate Chapin <japhet at chromium.org>'s
request for review:
Bug 39695: FrameLoader cleanup: Merge some bool members into a state machine
https://bugs.webkit.org/show_bug.cgi?id=39695

Attachment 57906: Fix chromium compile issue
https://bugs.webkit.org/attachment.cgi?id=57906&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
Just naming nits:

WebCore/loader/FrameLoader.cpp:211
 +	, m_loaderState(new FrameLoaderState())
We probably don't need this malloc here.

WebCore/loader/FrameLoaderState.h:38
 +  class FrameLoaderState : public Noncopyable {
Sorry to nit pick you on names, but how about FrameLoaderStateMachine ?

WebCore/loader/FrameLoader.h:313
 +	FrameLoaderState* loaderState() const { return m_loaderState.get(); }
Then you could name this method stateMachine().  The problem is that
FrameLoaderState doesn't hold all the state of the FrameLoader.  It just holds
the state machine state.

WebCore/loader/FrameLoader.h:519
 +	OwnPtr<FrameLoaderState> m_loaderState;
If you look elsewhere in this class, you'll see some mutable member variables. 
That's better than holding these subobjects as pointers because we don't need
to malloc them, etc.