Make WebChannelIPCTransport into a RenderFrameObserver

As of version 63, Chromium creates proxy frames also for the main frame in the
frame tree during cross-process navigations. This leads to a segmentation fault
in WebChannelIPCTransport because we assume that all main frames are local.

See https://crrev.com/27caae83cb530daaf49f9a38793e427cdf493a65 for details.

This patch refactors the renderer-side WebChannelIPCTransport from a
RenderViewObserver into a RenderFrameObserver, which prevents the segmentation
fault since the RenderFrameObserver is not created for proxy frames. Most likely
this would have to be done eventually anyway since the RenderView and
RenderViewObserver classes are deprecated and will likely be removed as part of
the Site Isolation project.

Installation is changed to follow Chromium's RenderFrameImpl in the sense of
performing the installation from RenderFrameObserver::DidClearWindowObject
instead of ContentRendererClient::RunScriptsAtDocumentStart. This has the
benefit of avoiding the ScriptForbiddenScope DCHECK.

Additionally there are the following minor changes:

  - The deprecated parameterless version of v8::Value::ToObject() method is
    replaced with v8::Value::IsObject() check and v8::Local::Cast.

  - The deprecated v8::Handle typedef is replaced with v8::Local.

  - The deprecated single-parameter WebContentsObserver::OnMessageReceived is
    replaced with the new two-parameter version.

  - blink::MainThreadIsolate() is used instead of v8::Isolate::GetCurrent() for
    Install/Uninstall since we know we are executing on the main thread.

  - WebChannelIPCTransportHost is changed to ignore messages from unexpected
    renderers in case something goes wrong with the renderers.

  - Logging is added to WebChannelIPCTransportHost for debugging purposes.

Some new unit tests are added, all of which fail with the old version.

Task-number: QTBUG-66333
Change-Id: I936d142fb042d9f936a3f9d08d4328ecba595f1f
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

1 files changed, 116 insertions, 121 deletions

diff --git a/src/core/renderer/web_channel_ipc_transport.cpp b/src/core/renderer/web_channel_ipc_transport.cpp
index 534ee302d..bb544168f 100644
--- a/src/core/renderer/web_channel_ipc_transport.cpp
+++ b/src/core/renderer/web_channel_ipc_transport.cpp
@@ -45,13 +45,12 @@
 #include "common/qt_messages.h"
 
 #include "content/public/renderer/render_frame.h"
-#include "content/public/renderer/render_view.h"
 #include "gin/arguments.h"
 #include "gin/handle.h"
 #include "gin/object_template_builder.h"
 #include "gin/wrappable.h"
+#include "third_party/WebKit/public/web/WebKit.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
-#include "third_party/WebKit/public/web/WebView.h"
 #include "v8/include/v8.h"
 
 #include <QJsonDocument>
@@ -61,193 +60,189 @@ namespace QtWebEngineCore {
 class WebChannelTransport : public gin::Wrappable<WebChannelTransport> {
 public:
     static gin::WrapperInfo kWrapperInfo;
-    static void Install(blink::WebFrame *frame, uint worldId);
-    static void Uninstall(blink::WebFrame *frame, uint worldId);
+    static void Install(blink::WebLocalFrame *frame, uint worldId);
+    static void Uninstall(blink::WebLocalFrame *frame, uint worldId);
 private:
-    content::RenderView *GetRenderView(v8::Isolate *isolate);
-    WebChannelTransport() { }
-    gin::ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate *isolate) override;
+    WebChannelTransport() {}
+    bool NativeQtSendMessage(gin::Arguments *args);
 
-    bool NativeQtSendMessage(gin::Arguments *args)
-    {
-        content::RenderView *renderView = GetRenderView(args->isolate());
-        if (!renderView || args->Length() != 1)
-            return false;
-        v8::Handle<v8::Value> val;
-        args->GetNext(&val);
-        if (!val->IsString() && !val->IsStringObject())
-            return false;
-        v8::String::Utf8Value utf8(val->ToString());
-
-        QByteArray valueData(*utf8, utf8.length());
-        QJsonParseError error;
-        QJsonDocument doc = QJsonDocument::fromJson(valueData, &error);
-        if (error.error != QJsonParseError::NoError) {
-            qWarning("%s %d: Parsing error: %s",__FILE__, __LINE__, qPrintable(error.errorString()));
-            return false;
-        }
-        int size = 0;
-        const char *rawData = doc.rawData(&size);
-        if (size == 0)
-            return false;
-        renderView->Send(new WebChannelIPCTransportHost_SendMessage(renderView->GetRoutingID(), std::vector<char>(rawData, rawData + size)));
-        return true;
-    }
+    // gin::WrappableBase
+    gin::ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate *isolate) override;
 
     DISALLOW_COPY_AND_ASSIGN(WebChannelTransport);
 };
 
 gin::WrapperInfo WebChannelTransport::kWrapperInfo = { gin::kEmbedderNativeGin };
 
-void WebChannelTransport::Install(blink::WebFrame *frame, uint worldId)
+void WebChannelTransport::Install(blink::WebLocalFrame *frame, uint worldId)
 {
-    v8::Isolate *isolate = v8::Isolate::GetCurrent();
+    v8::Isolate *isolate = blink::MainThreadIsolate();
     v8::HandleScope handleScope(isolate);
-    v8::Handle<v8::Context> context;
+    v8::Local<v8::Context> context;
     if (worldId == 0)
-        context = frame->ToWebLocalFrame()->MainWorldScriptContext();
+        context = frame->MainWorldScriptContext();
     else
-        context = frame->ToWebLocalFrame()->IsolatedWorldScriptContext(worldId);
+        context = frame->IsolatedWorldScriptContext(worldId);
     v8::Context::Scope contextScope(context);
 
     gin::Handle<WebChannelTransport> transport = gin::CreateHandle(isolate, new WebChannelTransport);
-    v8::Handle<v8::Object> global = context->Global();
-    v8::Handle<v8::Object> qt = global->Get(gin::StringToV8(isolate, "qt"))->ToObject();
-    if (qt.IsEmpty()) {
-        qt = v8::Object::New(isolate);
-        global->Set(gin::StringToV8(isolate, "qt"), qt);
+
+    v8::Local<v8::Object> global = context->Global();
+    v8::Local<v8::Value> qtObjectValue = global->Get(gin::StringToV8(isolate, "qt"));
+    v8::Local<v8::Object> qtObject;
+    if (qtObjectValue.IsEmpty() || !qtObjectValue->IsObject()) {
+        qtObject = v8::Object::New(isolate);
+        global->Set(gin::StringToV8(isolate, "qt"), qtObject);
+    } else {
+        qtObject = v8::Local<v8::Object>::Cast(qtObjectValue);
     }
-    qt->Set(gin::StringToV8(isolate, "webChannelTransport"), transport.ToV8());
+    qtObject->Set(gin::StringToV8(isolate, "webChannelTransport"), transport.ToV8());
 }
 
-void WebChannelTransport::Uninstall(blink::WebFrame *frame, uint worldId)
+void WebChannelTransport::Uninstall(blink::WebLocalFrame *frame, uint worldId)
 {
-    v8::Isolate *isolate = v8::Isolate::GetCurrent();
+    v8::Isolate *isolate = blink::MainThreadIsolate();
     v8::HandleScope handleScope(isolate);
-    v8::Handle<v8::Context> context;
+    v8::Local<v8::Context> context;
     if (worldId == 0)
-        context = frame->ToWebLocalFrame()->MainWorldScriptContext();
+        context = frame->MainWorldScriptContext();
     else
-        context = frame->ToWebLocalFrame()->IsolatedWorldScriptContext(worldId);
+        context = frame->IsolatedWorldScriptContext(worldId);
     v8::Context::Scope contextScope(context);
 
-    v8::Handle<v8::Object> global(context->Global());
-    v8::Handle<v8::Object> qt = global->Get(gin::StringToV8(isolate, "qt"))->ToObject();
-    if (qt.IsEmpty())
+    v8::Local<v8::Object> global(context->Global());
+    v8::Local<v8::Value> qtObjectValue = global->Get(gin::StringToV8(isolate, "qt"));
+    if (qtObjectValue.IsEmpty() || !qtObjectValue->IsObject())
         return;
-    qt->Delete(gin::StringToV8(isolate, "webChannelTransport"));
-}
-
-gin::ObjectTemplateBuilder WebChannelTransport::GetObjectTemplateBuilder(v8::Isolate *isolate)
-{
-    return gin::Wrappable<WebChannelTransport>::GetObjectTemplateBuilder(isolate).SetMethod("send", &WebChannelTransport::NativeQtSendMessage);
+    v8::Local<v8::Object> qtObject = v8::Local<v8::Object>::Cast(qtObjectValue);
+    qtObject->Delete(gin::StringToV8(isolate, "webChannelTransport"));
 }
 
-content::RenderView *WebChannelTransport::GetRenderView(v8::Isolate *isolate)
+bool WebChannelTransport::NativeQtSendMessage(gin::Arguments *args)
 {
-    blink::WebLocalFrame *webframe = blink::WebLocalFrame::FrameForContext(isolate->GetCurrentContext());
-    DCHECK(webframe) << "There should be an active frame since we just got a native function called.";
-    if (!webframe)
-        return 0;
+    blink::WebLocalFrame *frame = blink::WebLocalFrame::FrameForCurrentContext();
+    if (!frame || !frame->View())
+        return false;
+
+    content::RenderFrame *renderFrame = content::RenderFrame::FromWebFrame(frame);
+    if (!renderFrame)
+        return false;
+
+    std::string message;
+    if (!args->GetNext(&message))
+        return false;
+
+    QByteArray valueData(message.data(), message.size());
+    QJsonParseError error;
+    QJsonDocument doc = QJsonDocument::fromJson(valueData, &error);
+    if (error.error != QJsonParseError::NoError) {
+        LOG(WARNING) << "Parsing error: " << qPrintable(error.errorString());
+        return false;
+    }
 
-    blink::WebView *webview = webframe->View();
-    if (!webview)
-        return 0;  // can happen during closing
+    int size = 0;
+    const char *rawData = doc.rawData(&size);
+    if (size == 0)
+        return false;
 
-    return content::RenderView::FromWebView(webview);
+    renderFrame->Send(new WebChannelIPCTransportHost_SendMessage(
+                          renderFrame->GetRoutingID(),
+                          std::vector<char>(rawData, rawData + size)));
+    return true;
 }
 
-WebChannelIPCTransport::WebChannelIPCTransport(content::RenderView *renderView)
-    : content::RenderViewObserver(renderView)
-    , content::RenderViewObserverTracker<WebChannelIPCTransport>(renderView)
-    , m_installed(false)
-    , m_installedWorldId(0)
+gin::ObjectTemplateBuilder WebChannelTransport::GetObjectTemplateBuilder(v8::Isolate *isolate)
 {
+    return gin::Wrappable<WebChannelTransport>::GetObjectTemplateBuilder(isolate)
+        .SetMethod("send", &WebChannelTransport::NativeQtSendMessage);
 }
 
-void WebChannelIPCTransport::RunScriptsAtDocumentStart(content::RenderFrame *render_frame)
+WebChannelIPCTransport::WebChannelIPCTransport(content::RenderFrame *renderFrame)
+    : content::RenderFrameObserver(renderFrame)
 {
-    // JavaScript run before this point doesn't stick, and needs to be redone.
-    // ### FIXME: we should try no even installing before
-    if (m_installed && render_frame->IsMainFrame())
-        WebChannelTransport::Install(render_frame->GetWebFrame(), m_installedWorldId);
 }
 
-
-void WebChannelIPCTransport::installWebChannel(uint worldId)
+void WebChannelIPCTransport::setWorldId(base::Optional<uint> worldId)
 {
-    blink::WebView *webView = render_view()->GetWebView();
-    if (!webView)
+    if (m_worldId == worldId)
         return;
-    WebChannelTransport::Install(webView->MainFrame(), worldId);
-    m_installed = true;
-    m_installedWorldId = worldId;
-}
 
-void WebChannelIPCTransport::uninstallWebChannel(uint worldId)
-{
-    Q_ASSERT(worldId == m_installedWorldId);
-    blink::WebView *webView = render_view()->GetWebView();
-    if (!webView)
-        return;
-    WebChannelTransport::Uninstall(webView->MainFrame(), worldId);
-    m_installed = false;
+    if (m_worldId && m_canUseContext)
+        WebChannelTransport::Uninstall(render_frame()->GetWebFrame(), *m_worldId);
+
+    m_worldId = worldId;
+
+    if (m_worldId && m_canUseContext)
+        WebChannelTransport::Install(render_frame()->GetWebFrame(), *m_worldId);
 }
 
-void WebChannelIPCTransport::dispatchWebChannelMessage(const std::vector<char> &binaryJSON, uint worldId)
+void WebChannelIPCTransport::dispatchWebChannelMessage(const std::vector<char> &binaryJson, uint worldId)
 {
-    blink::WebView *webView = render_view()->GetWebView();
-    if (!webView)
-        return;
+    DCHECK(m_canUseContext);
+    DCHECK(m_worldId == worldId);
 
-    QJsonDocument doc = QJsonDocument::fromRawData(binaryJSON.data(), binaryJSON.size(), QJsonDocument::BypassValidation);
-    Q_ASSERT(doc.isObject());
+    QJsonDocument doc = QJsonDocument::fromRawData(binaryJson.data(), binaryJson.size(), QJsonDocument::BypassValidation);
+    DCHECK(doc.isObject());
     QByteArray json = doc.toJson(QJsonDocument::Compact);
 
-    v8::Isolate *isolate = v8::Isolate::GetCurrent();
+    blink::WebLocalFrame *frame = render_frame()->GetWebFrame();
+    v8::Isolate *isolate = blink::MainThreadIsolate();
     v8::HandleScope handleScope(isolate);
-    blink::WebFrame *frame = webView->MainFrame();
-    v8::Handle<v8::Context> context;
+    v8::Local<v8::Context> context;
     if (worldId == 0)
-        context = frame->ToWebLocalFrame()->MainWorldScriptContext();
+        context = frame->MainWorldScriptContext();
     else
-        context = frame->ToWebLocalFrame()->IsolatedWorldScriptContext(worldId);
+        context = frame->IsolatedWorldScriptContext(worldId);
     v8::Context::Scope contextScope(context);
 
-    v8::Handle<v8::Object> global(context->Global());
-    v8::Handle<v8::Value> qtObjectValue(global->Get(gin::StringToV8(isolate, "qt")));
-    if (!qtObjectValue->IsObject())
+    v8::Local<v8::Object> global(context->Global());
+    v8::Local<v8::Value> qtObjectValue(global->Get(gin::StringToV8(isolate, "qt")));
+    if (qtObjectValue.IsEmpty() || !qtObjectValue->IsObject())
         return;
-    v8::Handle<v8::Value> webChannelObjectValue(qtObjectValue->ToObject()->Get(gin::StringToV8(isolate, "webChannelTransport")));
-    if (!webChannelObjectValue->IsObject())
+    v8::Local<v8::Object> qtObject = v8::Local<v8::Object>::Cast(qtObjectValue);
+    v8::Local<v8::Value> webChannelObjectValue(qtObject->Get(gin::StringToV8(isolate, "webChannelTransport")));
+    if (webChannelObjectValue.IsEmpty() || !webChannelObjectValue->IsObject())
         return;
-    v8::Handle<v8::Value> onmessageCallbackValue(webChannelObjectValue->ToObject()->Get(gin::StringToV8(isolate, "onmessage")));
-    if (!onmessageCallbackValue->IsFunction()) {
-        qWarning("onmessage is not a callable property of qt.webChannelTransport. Some things might not work as expected.");
+    v8::Local<v8::Object> webChannelObject = v8::Local<v8::Object>::Cast(webChannelObjectValue);
+    v8::Local<v8::Value> callbackValue(webChannelObject->Get(gin::StringToV8(isolate, "onmessage")));
+    if (callbackValue.IsEmpty() || !callbackValue->IsFunction()) {
+        LOG(WARNING) << "onmessage is not a callable property of qt.webChannelTransport. Some things might not work as expected.";
         return;
     }
 
-    v8::Handle<v8::Object> messageObject(v8::Object::New(isolate));
+    v8::Local<v8::Object> messageObject(v8::Object::New(isolate));
     v8::Maybe<bool> wasSet = messageObject->DefineOwnProperty(
                 context,
                 v8::String::NewFromUtf8(isolate, "data"),
                 v8::String::NewFromUtf8(isolate, json.constData(), v8::String::kNormalString, json.size()),
                 v8::PropertyAttribute(v8::ReadOnly | v8::DontDelete));
-    Q_ASSERT(!wasSet.IsNothing() && wasSet.FromJust());
+    DCHECK(!wasSet.IsNothing() && wasSet.FromJust());
+
+    v8::Local<v8::Function> callback = v8::Local<v8::Function>::Cast(callbackValue);
+    v8::Local<v8::Value> argv[] = { messageObject };
+    frame->CallFunctionEvenIfScriptDisabled(callback, webChannelObject, 1, argv);
+}
+
+void WebChannelIPCTransport::WillReleaseScriptContext(v8::Local<v8::Context> context, int worldId)
+{
+    if (static_cast<uint>(worldId) == m_worldId)
+        m_canUseContext = false;
+}
 
-    v8::Handle<v8::Function> callback = v8::Handle<v8::Function>::Cast(onmessageCallbackValue);
-    const int argc = 1;
-    v8::Handle<v8::Value> argv[argc];
-    argv[0] = messageObject;
-    frame->ToWebLocalFrame()->CallFunctionEvenIfScriptDisabled(callback, webChannelObjectValue->ToObject(), argc, argv);
+void WebChannelIPCTransport::DidClearWindowObject()
+{
+    if (!m_canUseContext) {
+        m_canUseContext = true;
+        if (m_worldId)
+            WebChannelTransport::Install(render_frame()->GetWebFrame(), *m_worldId);
+    }
 }
 
 bool WebChannelIPCTransport::OnMessageReceived(const IPC::Message &message)
 {
     bool handled = true;
     IPC_BEGIN_MESSAGE_MAP(WebChannelIPCTransport, message)
-        IPC_MESSAGE_HANDLER(WebChannelIPCTransport_Install, installWebChannel)
-        IPC_MESSAGE_HANDLER(WebChannelIPCTransport_Uninstall, uninstallWebChannel)
+        IPC_MESSAGE_HANDLER(WebChannelIPCTransport_SetWorldId, setWorldId)
         IPC_MESSAGE_HANDLER(WebChannelIPCTransport_Message, dispatchWebChannelMessage)
         IPC_MESSAGE_UNHANDLED(handled = false)
     IPC_END_MESSAGE_MAP()
@@ -259,4 +254,4 @@ void WebChannelIPCTransport::OnDestruct()
     delete this;
 }
 
-} // namespace
+} // namespace QtWebEngineCore