[webkit-reviews] review granted: [Bug 191773] Web Inspector: "Reload Web Inspector" button no longer partially works : [Attachment 355173] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 191773: Web Inspector: "Reload Web Inspector" button no longer partially
works
https://bugs.webkit.org/show_bug.cgi?id=191773

Attachment 355173: [PATCH] Proposed Fix

https://bugs.webkit.org/attachment.cgi?id=355173&action=review




--- Comment #4 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 355173
  --> https://bugs.webkit.org/attachment.cgi?id=355173
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=355173&action=review

r=me

> Source/WebInspectorUI/ChangeLog:10
> +	   * UserInterface/Debug/UncaughtExceptionReporter.js:

I think there's also a `location.reload()`  inside Bootstrap.js:39 (reload Web
Inspector keyboard shortcut).  Frankly, I've never used it (and didn't know it
existed till now), so I'd also be fine if you removed it.

> Source/WebInspectorUI/ChangeLog:12
> +	   (sheetElement.innerHTML.div):
> +	  
(let.dismissOptionHTML.loadCompleted.string_appeared_here.dt.A.frivolous.except
ion.will.not.stop.me.dt.dd.a):

lolwut

> Source/WebKit/UIProcess/RemoteWebInspectorProxy.cpp:99
> +    load(m_debuggableType, m_backendCommandsURL);

Should we add assertions that `m_debuggableType` and `m_backendCommandsURL` are
both valid?  `load` doesn't so maybe I'm just being paranoid...

> Source/WebKitLegacy/win/WebCoreSupport/WebInspectorClient.cpp:276
> +    if (Page* inspectedPage = m_inspectedWebView->page())

Not sure about our style on this, but IIRC this would be a great `auto*` case