Re: [webkit-dev] Is it OK to remove Frame::setIsDisconnected() and isDisconnected() ?
On Wed, Apr 15, 2009 at 2:21 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 15, 2009, at 1:29 PM, Sverrir Á. Berg wrote: Hi Adam, Thanks for the links. These are simply exposing the functions as a formal a API's. I understand that you typically don't want to change externally exposed API's but these can easily be stubbed out (or removed). I should have pointed out in my original email that I have tried to remove these API's and I can still run all the WebKit/Mac tests fine. So at least two things are missing (IMHO) - tests that verify that this functionality is working as intended and documentation to tell what that intent is. But this is only required if somebody is actually using these functions... The API (system-private API actually, or SPI as we call it) in question is used by Safari. I don't believe it would be safe to remove this flag. The intent of the WebFrame API is that you can make a frame act (from the point of view of content inside it) as if it is a top-level frame, even though it is actually a subframe. One case where this might be useful is if you wanted to build a custom browser-like UI partially using HTML, which could then load pages as subframes without exposing that fact. It is true and unfortunate that we don't have tests - the ObjC API does not have as much test coverage as features exposed to Web content. Pardon my thread necromancy, but is this SPI still used by Safari? A brief read through the code turns up tons of bugs related to this feature. Are these bugs important to fix? Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Is it OK to remove Frame::setIsDisconnected() and isDisconnected() ?
On Apr 9, 2012, at 12:27 PM, Adam Barth wrote: On Wed, Apr 15, 2009 at 2:21 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 15, 2009, at 1:29 PM, Sverrir Á. Berg wrote: Hi Adam, Thanks for the links. These are simply exposing the functions as a formal a API's. I understand that you typically don't want to change externally exposed API's but these can easily be stubbed out (or removed). I should have pointed out in my original email that I have tried to remove these API's and I can still run all the WebKit/Mac tests fine. So at least two things are missing (IMHO) - tests that verify that this functionality is working as intended and documentation to tell what that intent is. But this is only required if somebody is actually using these functions... The API (system-private API actually, or SPI as we call it) in question is used by Safari. I don't believe it would be safe to remove this flag. The intent of the WebFrame API is that you can make a frame act (from the point of view of content inside it) as if it is a top-level frame, even though it is actually a subframe. One case where this might be useful is if you wanted to build a custom browser-like UI partially using HTML, which could then load pages as subframes without exposing that fact. It is true and unfortunate that we don't have tests - the ObjC API does not have as much test coverage as features exposed to Web content. Pardon my thread necromancy, but is this SPI still used by Safari? Yes. It is used by the Reader feature of Safari. A brief read through the code turns up tons of bugs related to this feature. Are these bugs important to fix? Hard to say, without knowing what the specific bugs are. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Is it OK to remove Frame::setIsDisconnected() and isDisconnected() ?
On Mon, Apr 9, 2012 at 3:24 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 9, 2012, at 12:27 PM, Adam Barth wrote: On Wed, Apr 15, 2009 at 2:21 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 15, 2009, at 1:29 PM, Sverrir Á. Berg wrote: Hi Adam, Thanks for the links. These are simply exposing the functions as a formal a API's. I understand that you typically don't want to change externally exposed API's but these can easily be stubbed out (or removed). I should have pointed out in my original email that I have tried to remove these API's and I can still run all the WebKit/Mac tests fine. So at least two things are missing (IMHO) - tests that verify that this functionality is working as intended and documentation to tell what that intent is. But this is only required if somebody is actually using these functions... The API (system-private API actually, or SPI as we call it) in question is used by Safari. I don't believe it would be safe to remove this flag. The intent of the WebFrame API is that you can make a frame act (from the point of view of content inside it) as if it is a top-level frame, even though it is actually a subframe. One case where this might be useful is if you wanted to build a custom browser-like UI partially using HTML, which could then load pages as subframes without exposing that fact. It is true and unfortunate that we don't have tests - the ObjC API does not have as much test coverage as features exposed to Web content. Pardon my thread necromancy, but is this SPI still used by Safari? Yes. It is used by the Reader feature of Safari. A brief read through the code turns up tons of bugs related to this feature. Are these bugs important to fix? Hard to say, without knowing what the specific bugs are. Ok. I'll file the ones I noticed. Thanks! Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Is it OK to remove Frame::setIsDisconnected() and isDisconnected() ?
On Apr 15, 2009, at 3:48 PM, Sverrir Á. Berg wrote: Working on a change in FrameTree and noticed that the checks in top() and parent() for 'checkForDisconnectedFrame' rely on a flag in Frame that as far as I can tell is never set. So my naive question is: Can I remove the corresponding code from Frame and FrameTree? If not I would like if somebody could explain how they are used so I don't break anything with my change. More detail: * My search for calls to Frame::setIsDisconnected reveals no callers in WebKit, Chromium or Google code search. There are two callers: http://trac.webkit.org/browser/trunk/WebKit/mac/WebView/WebFrame.mm?rev=42451#L1115 and http://trac.webkit.org/browser/trunk/WebKit/win/WebFrame.cpp?rev=42451#L283 -Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Is it OK to remove Frame::setIsDisconnected() and isDisconnected() ?
Hi Adam,Thanks for the links. These are simply exposing the functions as a formal a API's. I understand that you typically don't want to change externally exposed API's but these can easily be stubbed out (or removed). I should have pointed out in my original email that I have tried to remove these API's and I can still run all the WebKit/Mac tests fine. So at least two things are missing (IMHO) - tests that verify that this functionality is working as intended and documentation to tell what that intent is. But this is only required if somebody is actually using these functions... Thanks, Sverrir On Wed, Apr 15, 2009 at 4:17 PM, Adam Roben aro...@apple.com wrote: On Apr 15, 2009, at 3:48 PM, Sverrir Á. Berg wrote: Working on a change in FrameTree and noticed that the checks in top() and parent() for 'checkForDisconnectedFrame' rely on a flag in Frame that as far as I can tell is never set. So my naive question is: Can I remove the corresponding code from Frame and FrameTree? If not I would like if somebody could explain how they are used so I don't break anything with my change. More detail: * My search for calls to Frame::setIsDisconnected reveals no callers in WebKit, Chromium or Google code search. There are two callers: http://trac.webkit.org/browser/trunk/WebKit/mac/WebView/WebFrame.mm?rev=42451#L1115 and http://trac.webkit.org/browser/trunk/WebKit/win/WebFrame.cpp?rev=42451#L283 -Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Is it OK to remove Frame::setIsDisconnected() and isDisconnected() ?
On Apr 15, 2009, at 1:29 PM, Sverrir Á. Berg wrote: Hi Adam, Thanks for the links. These are simply exposing the functions as a formal a API's. I understand that you typically don't want to change externally exposed API's but these can easily be stubbed out (or removed). I should have pointed out in my original email that I have tried to remove these API's and I can still run all the WebKit/Mac tests fine. So at least two things are missing (IMHO) - tests that verify that this functionality is working as intended and documentation to tell what that intent is. But this is only required if somebody is actually using these functions... The API (system-private API actually, or SPI as we call it) in question is used by Safari. I don't believe it would be safe to remove this flag. The intent of the WebFrame API is that you can make a frame act (from the point of view of content inside it) as if it is a top- level frame, even though it is actually a subframe. One case where this might be useful is if you wanted to build a custom browser-like UI partially using HTML, which could then load pages as subframes without exposing that fact. It is true and unfortunate that we don't have tests - the ObjC API does not have as much test coverage as features exposed to Web content. - Maciej Thanks, Sverrir On Wed, Apr 15, 2009 at 4:17 PM, Adam Roben aro...@apple.com wrote: On Apr 15, 2009, at 3:48 PM, Sverrir Á. Berg wrote: Working on a change in FrameTree and noticed that the checks in top() and parent() for 'checkForDisconnectedFrame' rely on a flag in Frame that as far as I can tell is never set. So my naive question is: Can I remove the corresponding code from Frame and FrameTree? If not I would like if somebody could explain how they are used so I don't break anything with my change. More detail: * My search for calls to Frame::setIsDisconnected reveals no callers in WebKit, Chromium or Google code search. There are two callers: http://trac.webkit.org/browser/trunk/WebKit/mac/WebView/WebFrame.mm?rev=42451#L1115 and http://trac.webkit.org/browser/trunk/WebKit/win/WebFrame.cpp?rev=42451#L283 -Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
