Detach old frame on WebFrame::swap.

components/html_viewer/html_document.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_HTML_VIEWER_HTML_DOCUMENT_H_
 #define COMPONENTS_HTML_VIEWER_HTML_DOCUMENT_H_
 
 #include <set>
 
 #include "base/callback.h"
@@ skipping 70 matching lines @@
   virtual blink::WebMediaPlayer* createMediaPlayer(
       blink::WebLocalFrame* frame,
       const blink::WebURL& url,
       blink::WebMediaPlayerClient* client,
       blink::WebContentDecryptionModule* initial_cdm);
   virtual blink::WebFrame* createChildFrame(
       blink::WebLocalFrame* parent,
       blink::WebTreeScopeType scope,
       const blink::WebString& frameName,
       blink::WebSandboxFlags sandboxFlags);
-  virtual void frameDetached(blink::WebFrame*);
+  virtual void frameDetached(blink::WebFrame* frame);
+  virtual void frameDetached(blink::WebFrame* frame, DetachType type);

[Charlie Reis, 2015/06/01 23:41:46]

This doesn't look right to me for two reasons.

1) dcheng, why is it right to omit the qualifiers?  All the other Blink types
are prefixed with blink::, here and elsewhere.  I'm not 100% sure on our policy
here, but it seems hard for readers to know where this is coming from
(especially with another DetachType defined in browser.h).

2) There's no DetachType defined anywhere I can find.  The corresponding Blink
CL introduces WebDetachReason in WebFrameClient, but I thought this CL was a
prerequisite for that one?

[dcheng, 2015/06/02 00:21:51]

There is no special rule for Blink types. In general, Chrome code only qualify
names as much as needed: for example, developers are encouraged to write
RenderFrameHost inside namespace content { } rather than
content::RenderFrameHost. There are similar examples for subclasses of
interfaces following this principle as well:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/dow...
is one: the enum DangerousDownloadAction is defined inside the
content::DownloadTestObserver, but the constructor calls it without any
qualifiers.

Similarly, the qualifiers here are redundant, so we omit them to be consistent.
DetachType is defined inside WebFrameClient, and inside WebFrameClient and its
subclasses, the meaning of DetachType is not ambiguous. Outside of
WebFrameClient, full qualification of the identifier will be required,
preventing potential ambiguity.
It's the other way around.

[Charlie Reis, 2015/06/02 17:05:27]

Ah, it's because all of these are subclasses of WebFrameClient.  I'm ok with
that.  (It seemed random to prefix some Blink types but not others, but now I
see the reasoning.)
Oh, there was another CL.  The one I mentioned (linked from the description) is
meant to land after this.

   virtual blink::WebCookieJar* cookieJar(blink::WebLocalFrame* frame);
   virtual blink::WebNavigationPolicy decidePolicyForNavigation(
       const NavigationPolicyInfo& info);
 
   virtual void didAddMessageToConsole(const blink::WebConsoleMessage& message,
                                       const blink::WebString& source_name,
                                       unsigned source_line,
                                       const blink::WebString& stack_trace);
   virtual void didFinishLoad(blink::WebLocalFrame* frame);
   virtual void didNavigateWithinPage(blink::WebLocalFrame* frame,
@@ skipping 46 matching lines @@
   Setup* setup_;
 
   scoped_ptr<TouchHandler> touch_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(HTMLDocument);
 };
 
 }  // namespace html_viewer
 
 #endif  // COMPONENTS_HTML_VIEWER_HTML_DOCUMENT_H_