Separate page instance is now used to render overlay. Paused in debugger is migrated to the new overlay.
Created attachment 160705 [details] Patch
Comment on attachment 160705 [details] Patch Attachment 160705 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13620108
Comment on attachment 160705 [details] Patch Attachment 160705 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13623008
Comment on attachment 160705 [details] Patch Attachment 160705 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13608409
Comment on attachment 160705 [details] Patch Attachment 160705 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13609345
Comment on attachment 160705 [details] Patch Attachment 160705 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13616168
Created attachment 160710 [details] Patch
Comment on attachment 160710 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160710&action=review > Source/WebCore/inspector/InspectorOverlay.h:111 > + void drawNodeHighlight(GraphicsContext*); > + void drawRectHighlight(GraphicsContext*); > + void drawOverlayPage(GraphicsContext*); Why did this change? Does it have to be a part of this change? > Source/WebCore/inspector/InspectorOverlayPage.html:59 > +body { > + margin: 0; > + padding: 0; > +} > + > +fill { > + position: absolute; > + top: 0; > + right: 0; > + bottom: 0; > + left: 0; > +} > + > +.dimmed { > + background-color: rgba(0, 0, 0, 0.31); > +} > + > +.message-line { > + text-align: center; > +} > + > +.message-box { > + font-family: monospace; > + background-color: rgb(255, 255, 194); > + border: 1px solid rgb(128, 128, 128); > + display: inline-block; > + margin: 10px 0; > + padding: 2px 4px; > +} 4 spaces pls.
Comment on attachment 160710 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160710&action=review > Source/WebCore/inspector/InspectorOverlay.cpp:604 > + stateSaver.restore(); redundant? > Source/WebCore/inspector/InspectorOverlay.cpp:658 > + overlayPage()->mainFrame()->script()->evaluate(ScriptSourceCode(makeString(method, "(\"", argument, "\")"))); What if argument contains quotes?
Comment on attachment 160710 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160710&action=review > Source/WebCore/inspector/InspectorOverlayPage.html:1 > +<!-- You may want to have DOCTYPE declaration here to avoid surprises from quirks mode.
Created attachment 160718 [details] Patch
Comment on attachment 160718 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160718&action=review > Source/WebCore/bindings/js/ScriptController.h:29 > +#include "ScriptValue.h" Why did this change? > Source/WebCore/inspector/InspectorOverlay.cpp:605 > + stateSaver.restore(); Remove this line, ~GraphicsContextStateSaver will do this for you. > Source/WebCore/inspector/InspectorOverlay.cpp:650 > + loader->activeDocumentLoader()->writer()->begin(KURL()); Please call begin() instead. > Source/WebCore/inspector/InspectorOverlayPage.html:86 > +<canvas id="canvas" class="fill"></canvas> It seems to be unused, please remove.
Created attachment 160902 [details] Patch
(In reply to comment #12) > (From update of attachment 160718 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=160718&action=review > > > Source/WebCore/bindings/js/ScriptController.h:29 > > +#include "ScriptValue.h" > > Why did this change? > ScriptController's evaluate returns ScriptValue that is forward-declared. But that return value is not always used. As a result, the call site for frame->script()->evaluate("Foo") needs to include "ScriptValue.h". I thought it made ore sense to include it from the ScriptController to avoid further confusion. Rolled back since it raised questions. > > Source/WebCore/inspector/InspectorOverlay.cpp:605 > > + stateSaver.restore(); > > Remove this line, ~GraphicsContextStateSaver will do this for you. > Done. > > Source/WebCore/inspector/InspectorOverlay.cpp:650 > > + loader->activeDocumentLoader()->writer()->begin(KURL()); > > Please call begin() instead. > Done. > > Source/WebCore/inspector/InspectorOverlayPage.html:86 > > +<canvas id="canvas" class="fill"></canvas> > > It seems to be unused, please remove. Done.
Created attachment 160907 [details] Patch
(In reply to comment #14) > (In reply to comment #12) > > (From update of attachment 160718 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=160718&action=review > > > > > Source/WebCore/bindings/js/ScriptController.h:29 > > > +#include "ScriptValue.h" > > > > Why did this change? > > > > ScriptController's evaluate returns ScriptValue that is forward-declared. But that return value is not always used. As a result, the call site for frame->script()->evaluate("Foo") needs to include "ScriptValue.h". I thought it made ore sense to include it from the ScriptController to avoid further confusion. Rolled back since it raised questions. > I believe we generally include headers on the caller site to avoid increasing number of #includes in the header files and so compilation time.
Committed r126857: <http://trac.webkit.org/changeset/126857>