inspector: Refactor how opening/closing work

The inspector object is now re-used and only created once (when the inspector
is opened first). This also moves some logic from CommandDispatcher to the
inspector object.

The geometry is now saved again when the window is closed (via closeEvent)
rather than in AbstractWebInspector.set_position - after all, the window will
also close when the position is changed.

Finally, set_position(None) changed its meaning: Its now not used anymore to
close/delete the inspector, but instead means "use the last saved position".

3 files changed, 38 insertions, 52 deletions

diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py
index 62fa3df7c..e464de6a2 100644
--- a/qutebrowser/browser/browsertab.py
+++ b/qutebrowser/browser/browsertab.py
@@ -124,7 +124,7 @@ class TabData:
         fullscreen: Whether the tab has a video shown fullscreen currently.
         netrc_used: Whether netrc authentication was performed.
         input_mode: current input mode for the tab.
-        splitter: InspectorSplitter used to show inspector inside the tab
+        splitter: InspectorSplitter used to show inspector inside the tab.
     """
 
     keep_icon = attr.ib(False)  # type: bool
diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py
index 70c9ad7a6..ab1dc0699 100644
--- a/qutebrowser/browser/commands.py
+++ b/qutebrowser/browser/commands.py
@@ -1259,31 +1259,13 @@ class CommandDispatcher:
         # FIXME:qtwebengine have a proper API for this
         page = tab._widget.page()  # pylint: disable=protected-access
 
-        @pyqtSlot()
-        def _on_inspector_closed():
-            tab.data.inspector = None
-
         try:
             if tab.data.inspector is None:
-                if position is None:
-                    try:
-                        position = inspector.Position[
-                            configfiles.state['general'][
-                                'inspector_last_position']]
-                    except KeyError:
-                        position = inspector.Position.right
-
                 tab.data.inspector = inspector.create(
-                    splitter=tab.data.splitter, win_id=tab.win_id)
+                    splitter=tab.data.splitter,
+                    win_id=tab.win_id)
                 tab.data.inspector.inspect(page)
-                tab.data.inspector.closed.connect(_on_inspector_closed)
-
             tab.data.inspector.set_position(position)
-
-            if position:
-                configfiles.state['general'][
-                    'inspector_last_position'] = position.name
-
         except inspector.WebInspectorError as e:
             raise cmdutils.CommandError(e)
 
diff --git a/qutebrowser/browser/inspector.py b/qutebrowser/browser/inspector.py
index a4f46e35a..f6a62eba8 100644
--- a/qutebrowser/browser/inspector.py
+++ b/qutebrowser/browser/inspector.py
@@ -105,13 +105,8 @@ class AbstractWebInspector(QWidget):
     Attributes:
         _position: position of the inspector (right/left/top/bottom/window)
         _splitter: InspectorSplitter where the inspector can be placed.
-
-    Signals:
-        closed: Emitted when the inspector is closed.
     """
 
-    closed = pyqtSignal()
-
     def __init__(self, splitter: 'miscwidgets.InspectorSplitter',
                  win_id: int,
                  parent: QWidget = None) -> None:
@@ -131,28 +126,42 @@ class AbstractWebInspector(QWidget):
         self._layout.wrap(self, widget)
         self._widget.installEventFilter(self._child_event_filter)
 
+    def _last_position(self) -> Position:
+        """Get the last position the inspector was in."""
+        try:
+            pos = configfiles.state['general']['inspector_last_position']
+        except KeyError:
+            return Position.right
+        else:
+            return Position[pos]
+
+    def _save_last_position(self, position: Position) -> None:
+        """Save the last position the inspector was in."""
+        configfiles.state['general']['inspector_last_position'] = position.name
+
     def set_position(self, position: typing.Optional[Position]) -> None:
         """Set the position of the inspector.
 
-        Will close the inspector if position is None."""
-        if position != self._position:
-            if self._position == Position.window:
-                self._save_state_geometry()
-
-            self._position = position
-
-            if position is None:
-                self.hide()
-                self.deleteLater()
-                self.closed.emit()
-            elif position == Position.window:
-                self.hide()
-                self.setWindowTitle("Web Inspector")
-                self.setParent(None)  # type: ignore
-                self._load_state_geometry()
-            else:
-                self._splitter.set_inspector(self, position)
-            self.show()
+        If the position is None, the last known position is used.
+        """
+        if position is None:
+            position = self._last_position()
+        else:
+            self._save_last_position(position)
+
+        if position == self._position:
+            self.setVisible(not self.isVisible())
+            return
+
+        self._position = position
+
+        if position == Position.window:
+            self.setParent(None)  # type: ignore
+            self.setWindowTitle("Web Inspector")
+            self._load_state_geometry()
+        else:
+            self._splitter.set_inspector(self, position)
+        self.show()
 
     def _load_state_geometry(self) -> None:
         """Load the geometry from the state file."""
@@ -170,17 +179,12 @@ class AbstractWebInspector(QWidget):
             if not ok:
                 log.init.warning("Error while loading geometry.")
 
-    def _save_state_geometry(self) -> None:
-        """Save the geometry to the state file."""
+    def closeEvent(self, e: QCloseEvent) -> None:
+        """Save the geometry when closed."""
         data = self.saveGeometry().data()
         geom = base64.b64encode(data).decode('ASCII')
         configfiles.state['geometry']['inspector'] = geom
 
-    def closeEvent(self, e: QCloseEvent) -> None:
-        """Save the window geometry when closed."""
-        self.set_position(None)
-        super().closeEvent(e)
-
     def inspect(self, page: QWidget) -> None:
         """Inspect the given QWeb(Engine)Page."""
         raise NotImplementedError