Get rid of all code in qt/core/glue

Bug #1435418 reported by Chris Coulson on 2015-03-23
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Oxide
Low
Chris Coulson

Bug Description

I've tried to rearrange things to make it clear how code should be split between, say, WebViewAdapter in qt/core/glue and WebView in qt/core/browser, but the code split still isn't really all that clear (even to me). In addition to that, there's other problems:

- There's a lot of implementation details exposed between the 2 directories, using private methods and friend statements which all seems a bit wrong.
- Using the WebView case again, there may be a legitimate reason for another class in Oxide to do OxideQQuickWebViewPrivate::get() in order to access a subset of methods on OxideQQuickWebViewPrivate, but it also exposes the entire API surface of WebViewAdapter as its methods have to be public in order to be accessible from OxideQQuickWebView (unless we make them protected and use a friend statement).

I think we should refactor it all to do this instead (using webview as an example):

(sorry if this is not clear)

class OxideQQuickWebView : public QQuickItem {
 public:
  ...
  <public API>
  ...

 private:
  QScopedPointer<OxideQQuickWebViewPrivate> d_ptr;
};

class OxideQQuickWebViewPrivate : public oxide::qt::WebViewProxyClient {
 public:
  static OxideQQuickWebViewPrivate* get(OxideQQuickWebView* q);
  ...
  <methods available to other code in qt/quick/api>
  ...

 private:
  ...
  <implementation of WebViewProxyClient>
  ...

  QScopedPointer<oxide::qt::WebViewProxy> proxy_;
};

WebViewProxy and WebViewProxyClient are abstract interfaces in qt/core/glue, although WebViewProxy has a static create() function.

In qt/core/browser:

WebView : public WebViewProxy {
 public:
  ...
  <methods available to code in qt/core/browser>
  ...

 private:
  ...
  <implementation of WebViewProxy>
  ...

  WebViewProxyClient* client_;
};

This fixes the above problems (we don't have to expose implementation details between qt/core/browser and qt/core/glue, the WebViewProxy instance is private to OxideQQuickWebViewPrivate and its API isn't exposed to classes that call OxideQQuickWebViewPrivate::get(), and developers don't need to spend time trying to figure out which directory to put code in to)

Changed in oxide:
importance: Undecided → Low
status: New → Triaged
description: updated
Olivier Tilloy (osomon) wrote :

On the paper, looks good to me.

Changed in oxide:
assignee: nobody → Chris Coulson (chrisccoulson)
milestone: none → branch-1.7
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers