Require WebLocalFrame to be created with a non-null client

third_party/WebKit/public/web/WebFrameClient.h

Index: third_party/WebKit/public/web/WebFrameClient.h
diff --git a/third_party/WebKit/public/web/WebFrameClient.h b/third_party/WebKit/public/web/WebFrameClient.h
index e2c7a2075f5a16bb984f1a77121277ec2df68443..c0707d3172718d53d9a6a43ba1c5c263c6894cc2 100644
--- a/third_party/WebKit/public/web/WebFrameClient.h
+++ b/third_party/WebKit/public/web/WebFrameClient.h
@@ -83,6 +83,7 @@ class WebExternalPopupMenuClient;
 class WebFileChooserCompletion;
 class WebFormElement;
 class WebInstalledAppClient;
+class WebLocalFrame;
 class WebMediaPlayer;
 class WebMediaPlayerClient;
 class WebMediaPlayerEncryptedMediaClient;
@@ -108,8 +109,10 @@ struct WebPopupMenuInfo;
 struct WebRect;
 struct WebURLError;
 
-class WebFrameClient {
+class BLINK_EXPORT WebFrameClient {
 public:
+    virtual ~WebFrameClient() {}

[dcheng, 2016/09/29 04:13:28]

I'm not sure how I feel about making this public, since it forces all derived
classes to have a public dtor.

At the same time, having two copies of MockWebFrameClient lying around the code
base also seemed weird.

+
     // Factory methods -----------------------------------------------------
 
     // May return null.
@@ -156,13 +159,11 @@ public:
     // pending navigation's URL, because a URL spoof is possible.
     virtual void didAccessInitialDocument() { }
 
-    // A child frame was created in this frame. This is called when the frame
-    // is created and initialized. Takes the name of the new frame, the parent
-    // frame and returns a new WebFrame. The WebFrame is considered in-use
-    // until frameDetached() is called on it.
-    // Note: If you override this, you should almost certainly be overriding
-    // frameDetached().
-    virtual WebFrame* createChildFrame(WebLocalFrame* parent, WebTreeScopeType, const WebString& name, const WebString& uniqueName, WebSandboxFlags sandboxFlags, const WebFrameOwnerProperties&) { return nullptr; }
+    // Request the creation of a new child frame. Embedders may return nullptr
+    // to prevent the new child frame from being attached. Otherwise, embedders
+    // should create a new WebLocalFrame, insert it into the frame tree, and
+    // return the created frame.
+    virtual WebLocalFrame* createChildFrame(WebLocalFrame* parent, WebTreeScopeType, const WebString& name, const WebString& uniqueName, WebSandboxFlags sandboxFlags, const WebFrameOwnerProperties&) { return nullptr; }

[dcheng, 2016/09/29 04:13:28]

Changed the return type since all child frames start as local frames.

 
     // This frame has set its opener to another frame, or disowned the opener
     // if opener is null. See http://html.spec.whatwg.org/#dom-opener.
@@ -171,15 +172,17 @@ public:
     // Specifies the reason for the detachment.
     enum class DetachType { Remove, Swap };
 
-    // This frame has been detached from the view, but has not been closed yet.
-    virtual void frameDetached(WebLocalFrame*, DetachType) {}
+    // This frame has been detached. Embedders should release any resources
+    // associated with this frame. If the DetachType is Remove, the frame should
+    // also be removed from the frame tree; otherwise, if the DetachType is
+    // Swap, the frame is being replaced in-place by WebFrame::swap().
+    virtual void frameDetached(WebLocalFrame*, DetachType);

[dcheng, 2016/09/29 04:13:28]

I gave this a default implementation to avoid a bunch of boilerplate.

 
-    // This frame has become focused..
+    // This frame has become focused.
     virtual void frameFocused() { }
 
-    // This frame is about to be closed. This is called after frameDetached,
-    // when the document is being unloaded, due to new one committing.
-    virtual void willClose(WebFrame*) { }
+    // A provisional load is about to commit.
+    virtual void willCommitProvisionalLoad(WebLocalFrame*) {}

[dcheng, 2016/09/29 04:13:27]

I'm not sure how willClose() ended up meaning this, but the original name didn't
reflect its purpose at all, so I renamed it.

 
     // This frame's name has changed.
     virtual void didChangeName(const WebString& name, const WebString& uniqueName) { }
@@ -693,9 +696,6 @@ public:
     {
         return WebURL();
     }
-
-protected:
-    virtual ~WebFrameClient() { }
 };
 
 } // namespace blink