Bug 174662 – [Win] Implement Authentication dialog in MiniBrowser

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Basuke Suzuki]

There's no authentication dialog for Windows MiniBrowser.

[Basuke Suzuki]

Created attachment 315997 [details]
Implemented

[Build Bot]

Attachment 315997 [details] did not pass style-queue:


ERROR: Tools/MiniBrowser/win/Common.cpp:671:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Tools/MiniBrowser/win/Common.cpp:681:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:1:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Tools/MiniBrowser/win/ResourceLoadDelegate.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/MiniBrowser/win/ResourceLoadDelegate.cpp:97:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Tools/MiniBrowser/win/ResourceLoadDelegate.cpp:107:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 7 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Basuke Suzuki]

Created attachment 316005 [details]
Fix style

[Basuke Suzuki]

(In reply to Build Bot from comment #2)
> ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:0:  No copyright
> message found.  You should have a line: "Copyright [year] <Copyright Owner>"
> [legal/copyright] [5]
> ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:1:  Should have a
> space between // and comment  [whitespace/comments] [4]

This file is the one VisualStudio generates.

[Basuke Suzuki]

Created attachment 316007 [details]
Fix style

[Build Bot]

Attachment 316007 [details] did not pass style-queue:


ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:1:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 2 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 316007 [details]
Fix style

Attachment 316007 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4156792

New failing tests:
imported/w3c/IndexedDB-private-browsing/idbfactory_open12.html

[Build Bot]

Created attachment 316018 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Basuke Suzuki]

I have no idea why my change for Windows app break ios-sim layout test. Also I don't want to fix generated file because next modification causes that change useless.

[Alex Christensen]

Comment on attachment 316007 [details]
Fix style

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

iOS test failure unrelated

> Tools/MiniBrowser/win/ResourceLoadDelegate.cpp:40
> +extern HRESULT DisplayAuthDialog(std::wstring& username, std::wstring& password);

This should go in a header

[Basuke Suzuki]

(In reply to Alex Christensen from comment #10)
> Comment on attachment 316007 [details]
> Fix style
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316007&action=review
> 
> iOS test failure unrelated
> 
> > Tools/MiniBrowser/win/ResourceLoadDelegate.cpp:40
> > +extern HRESULT DisplayAuthDialog(std::wstring& username, std::wstring& password);
> 
> This should go in a header

I agree with that, but basic architecture of this app is very new to me which is very confusing. The "Common.cpp" file has no header file and directly included from "WinMain.cpp" so that no chance to get the definition easily. Other references to those defined method seems to be called inside Common.cpp as the result of event.

I am very new to Windows app programming and have no idea with this structure.

[Alex Christensen]

Comment on attachment 316007 [details]
Fix style

MiniBrowser isn't exactly the most well-organized project anyways.  Let's just land this.

[WebKit Commit Bot]

Comment on attachment 316007 [details]
Fix style

Clearing flags on attachment: 316007

Committed r219830: <http://trac.webkit.org/changeset/219830>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Basuke Suzuki]

(In reply to Alex Christensen from comment #12)
> Comment on attachment 316007 [details]
> Fix style
> 
> MiniBrowser isn't exactly the most well-organized project anyways.  Let's
> just land this.

Thanks!