Split javascript_dialogs.cc into views and cocoa+views versions.

Split javascript_dialogs.cc into views and cocoa+views versions.

This allows them both to be compiled on Mac, which is needed for
mac_views_browser (since it doesn't link any Cocoa code).

Compile step regressed at go/macviewsbuilder/builds/16832.

BUG=629964, 425229
Committed: https://crrev.com/49faa64fae69901c98ee47c81a4806b14e0a0e86
Cr-Commit-Position: refs/heads/master@{#428316}

[tapted, 2016-10-28 02:57:36]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 02:58:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2016-10-28 03:01:04]

Description was changed from

==========
Split javascript_dialogs.cc into views and cocoa+views versions.

This allows them both to be compiled on Mac, which is needed for
mac_views_browser (since it doesn't link any Cocoa code).

BUG=629964, 425229
==========

to

==========
Split javascript_dialogs.cc into views and cocoa+views versions.

This allows them both to be compiled on Mac, which is needed for
mac_views_browser (since it doesn't link any Cocoa code).

Compile step regressed at go/macviewsbuilder/builds/16832.

BUG=629964, 425229
==========

[tapted, 2016-10-28 03:01:05]

tapted@chromium.org changed reviewers:
+ avi@chromium.org

[tapted, 2016-10-28 04:02:10]

Hi Avi, please take a look

[commit-bot: I haz the power, 2016-10-28 04:12:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-28 04:12:09]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Avi (use Gerrit), 2016-10-28 04:27:50]

LGTM

I guess that works. Is there an #ifdef tho so you wouldn't have to do any of the
file work, but just do javascript_dialogs.cc:

#if defined(OS_MACOSX) && !MACVIEWS

or the like?

https://codereview.chromium.org/2458903002/diff/1/chrome/browser/ui/javascrip...
File chrome/browser/ui/javascript_dialogs/javascript_dialog.cc (right):

https://codereview.chromium.org/2458903002/diff/1/chrome/browser/ui/javascrip...
chrome/browser/ui/javascript_dialogs/javascript_dialog.cc:7: #include
"build/build_config.h"
you don't need this include then

https://codereview.chromium.org/2458903002/diff/1/chrome/browser/ui/javascrip...
File chrome/browser/ui/javascript_dialogs/javascript_dialog_mac.cc (right):

https://codereview.chromium.org/2458903002/diff/1/chrome/browser/ui/javascrip...
chrome/browser/ui/javascript_dialogs/javascript_dialog_mac.cc:7: #include
"build/build_config.h"
you don't need this

[tapted, 2016-10-28 04:38:20]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 04:38:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2016-10-28 04:48:25]

Thanks Avi!

On 2016/10/28 04:27:50, Avi wrote:
> I guess that works. Is there an #ifdef tho so you wouldn't have to do any of
the
> file work, but just do javascript_dialogs.cc:
> 
> #if defined(OS_MACOSX) && !MACVIEWS
> 
> or the like?

Not at this stage.. There are tradeoffs involved. #ifdefs can sometimes lead to
confusing code and weird build problems, and for Mac it's easy to fall into a
trap where you need to #include some ObjC code in a C++ file. It's also kinda
nice not having to recompile everything when switching mac_views_browser on/off
:).  On the other hand, understanding the build files and having the extra
discipline to structure things into separate files can be a burden.

I'm hesitant to make a MAC_VIEWS_BROWSER #define available while it's still not
widely understood what that is for (and doesn't have trybots, just an internal
builder that a few of us keep an eye on), but it could still happen. It's also
much easier rip out the code that's not needed later on (whichever direction
ends up being taken).

https://codereview.chromium.org/2458903002/diff/1/chrome/browser/ui/javascrip...
File chrome/browser/ui/javascript_dialogs/javascript_dialog.cc (right):

https://codereview.chromium.org/2458903002/diff/1/chrome/browser/ui/javascrip...
chrome/browser/ui/javascript_dialogs/javascript_dialog.cc:7: #include
"build/build_config.h"
On 2016/10/28 04:27:50, Avi wrote:
> you don't need this include then

Done.

https://codereview.chromium.org/2458903002/diff/1/chrome/browser/ui/javascrip...
File chrome/browser/ui/javascript_dialogs/javascript_dialog_mac.cc (right):

https://codereview.chromium.org/2458903002/diff/1/chrome/browser/ui/javascrip...
chrome/browser/ui/javascript_dialogs/javascript_dialog_mac.cc:7: #include
"build/build_config.h"
On 2016/10/28 04:27:50, Avi wrote:
> you don't need this

Done.

[commit-bot: I haz the power, 2016-10-28 06:00:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-28 06:00:43]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[tapted, 2016-10-28 07:39:21]

The CQ bit was checked by tapted@chromium.org

[tapted, 2016-10-28 07:39:21]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org
Link to the patchset: https://codereview.chromium.org/2458903002/#ps20001
(title: "-build_config.h")

[commit-bot: I haz the power, 2016-10-28 07:39:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 08:19:13]

Description was changed from

==========
Split javascript_dialogs.cc into views and cocoa+views versions.

This allows them both to be compiled on Mac, which is needed for
mac_views_browser (since it doesn't link any Cocoa code).

Compile step regressed at go/macviewsbuilder/builds/16832.

BUG=629964, 425229
==========

to

==========
Split javascript_dialogs.cc into views and cocoa+views versions.

This allows them both to be compiled on Mac, which is needed for
mac_views_browser (since it doesn't link any Cocoa code).

Compile step regressed at go/macviewsbuilder/builds/16832.

BUG=629964, 425229
==========

[commit-bot: I haz the power, 2016-10-28 08:19:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-28 08:21:00]

Description was changed from

==========
Split javascript_dialogs.cc into views and cocoa+views versions.

This allows them both to be compiled on Mac, which is needed for
mac_views_browser (since it doesn't link any Cocoa code).

Compile step regressed at go/macviewsbuilder/builds/16832.

BUG=629964, 425229
==========

to

==========
Split javascript_dialogs.cc into views and cocoa+views versions.

This allows them both to be compiled on Mac, which is needed for
mac_views_browser (since it doesn't link any Cocoa code).

Compile step regressed at go/macviewsbuilder/builds/16832.

BUG=629964, 425229

Committed: https://crrev.com/49faa64fae69901c98ee47c81a4806b14e0a0e86
Cr-Commit-Position: refs/heads/master@{#428316}
==========

[commit-bot: I haz the power, 2016-10-28 08:21:01]

Patchset 2 (id:??) landed as
https://crrev.com/49faa64fae69901c98ee47c81a4806b14e0a0e86
Cr-Commit-Position: refs/heads/master@{#428316}